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 665d1fa 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

Author: hlinnaka

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;