postgres.git / summary / log / commit / refs

commit    1ca312686e4c4b270cb598429983a07c73a5c44a
Author:   Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date:     Mon Oct 09 08:23:47 2023 +0000
First released in: 17.0

    Clarify the checks in RegisterBackgroundWorker.
    
    In EXEC_BACKEND or single-user mode, we process
    shared_preload_libraries at postmaster startup as usual, but also at
    backend startup. When a library calls RegisterBackgroundWorker() when
    being loaded into a backend process, we go through the motions to add
    the worker to BackgroundWorkerList, even though that is a
    postmaster-private data structure. Make it return early when called in
    a backend process, without changing BackgroundWorkerList.
    
    You could argue that it was intentional: In non-EXEC_BACKEND mode, the
    backend processes inherit BackgroundWorkerList at fork(), so it does
    make some sense to initialize it to the same state in EXEC_BACKEND
    mode, too. It's clearly a postmaster-private structure, though, and
    all the functions that use it are clearly marked as "should only be
    called in postmaster".
    
    You could also argue that libraries should not call
    RegisterBackgroundWorker() during backend startup. It's too late to
    correctly register any static background workers at that stage. But
    it's a common pattern in extensions, and it doesn't seem worth the
    churn to require all extensions to change it.
    
    Another sloppiness was the exception for "internal" background
    workers. We checked that RegisterBackgroundWorker() was called during
    shared_preload_libraries processing, or the background worker was an
    internal one. That exception was made in commit 665d1fad99 to allow
    postmaster to register the logical apply launcher in
    ApplyLauncherRegister(). The way the check was written, it would not
    complain if you registered an internal background worker in a regular
    backend process. But it would complain if postmaster registered a
    background worker defined in a shared library, outside
    shared_preload_libraries processing. I think the correct rule is that
    you can only register static background workers in the postmaster
    process, and only before the bgworker shared memory array has been
    initialized. Check for that more directly.
    
    Reviewed-by: Thomas Munro
    Discussion: https://www.postgresql.org/message-id/4f95c1fc-ad3c-7974-3a8c-6faa3931804c@iki.fi


 src/backend/postmaster/bgworker.c | 44 +++++++++++++++++++++++++++++----------
 1 file changed, 33 insertions(+), 11 deletions(-)

diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c
index 505e38376c3..31350d64013 100644
--- a/src/backend/postmaster/bgworker.c
+++ b/src/backend/postmaster/bgworker.c
@@ -880,21 +880,43 @@ RegisterBackgroundWorker(BackgroundWorker *worker)
 	RegisteredBgWorker *rw;
 	static int	numworkers = 0;
 
-	if (!IsUnderPostmaster)
-		ereport(DEBUG1,
-				(errmsg_internal("registering background worker \"%s\"", worker->bgw_name)));
-
-	if (!process_shared_preload_libraries_in_progress &&
-		strcmp(worker->bgw_library_name, "postgres") != 0)
+	/*
+	 * Static background workers can only be registered in the postmaster
+	 * process.
+	 */
+	if (IsUnderPostmaster || !IsPostmasterEnvironment)
 	{
-		if (!IsUnderPostmaster)
-			ereport(LOG,
-					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("background worker \"%s\": must be registered in shared_preload_libraries",
-							worker->bgw_name)));
+		/*
+		 * In EXEC_BACKEND or single-user mode, we process
+		 * shared_preload_libraries in backend processes too.  We cannot
+		 * register static background workers at that stage, but many
+		 * libraries' _PG_init() functions don't distinguish whether they're
+		 * being loaded in the postmaster or in a backend, they just check
+		 * process_shared_preload_libraries_in_progress.  It's a bit sloppy,
+		 * but for historical reasons we tolerate it.  In EXEC_BACKEND mode,
+		 * the background workers should already have been registered when the
+		 * library was loaded in postmaster.
+		 */
+		if (process_shared_preload_libraries_in_progress)
+			return;
+		ereport(LOG,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("background worker \"%s\": must be registered in shared_preload_libraries",
+						worker->bgw_name)));
 		return;
 	}
 
+	/*
+	 * Cannot register static background workers after calling
+	 * BackgroundWorkerShmemInit().
+	 */
+	if (BackgroundWorkerData != NULL)
+		elog(ERROR, "cannot register background worker \"%s\" after shmem init",
+			 worker->bgw_name);
+
+	ereport(DEBUG1,
+			(errmsg_internal("registering background worker \"%s\"", worker->bgw_name)));
+
 	if (!SanityCheckBackgroundWorker(worker, LOG))
 		return;
 


[parent: 608fd198def5]