Subject: Re: logical replication launcher crash on buildfarm
To: Robert Haas <robertmhaas@gmail.com>
References: <20170316032833.2zxfnqe3sl3ktrbt@alap3.anarazel.de>
 <20170316034246.3uurliezczmka6cn@alap3.anarazel.de>
 <e56bda7f-9d29-6900-0f90-96be8219b930@2ndquadrant.com>
 <20170316085322.crffknkgee5s6air@alap3.anarazel.de>
 <368e64f6-ee9a-f09f-82b4-a33b61b28d36@2ndquadrant.com>
 <20170327165023.t224nralrspk6udc@alap3.anarazel.de>
 <CA+Tgmoa3C7=gVvmwvvKWT6+mkm=XGSDqF4bXu6=fohArafWHMQ@mail.gmail.com>
 <52d78669-0523-4e60-a263-841c96210256@2ndquadrant.com>
 <8941c9b1-f6e9-1056-39f2-7bc7336c4d00@2ndquadrant.com>
 <20170328020422.i4srzzgq4zbzu2ci@alap3.anarazel.de>
 <CA+TgmobBTZ_eDbTmx-pfUndZDKPBAcj+r_3CQjEHFkp99kE2Yg@mail.gmail.com>
 <e3c35f3a-426d-b1ca-7fe5-9241c2717013@2ndquadrant.com>
 <CA+Tgmoae_obbYapxDhELe852TO1nFGOdVbWiUwa6Gf+ODxjHnQ@mail.gmail.com>
 <0471e978-b121-ad2f-2879-f1694d9a5ce7@2ndquadrant.com>
 <d217ee26-f3d1-4079-0110-3f7ee44ad6b4@2ndquadrant.com>
Cc: Andres Freund <andres@anarazel.de>,
 "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>,
 Petr Jelinek <petr@2ndquadrant.com>, Peter Eisentraut <peter_e@gmx.net>
From: Petr Jelinek <petr.jelinek@2ndquadrant.com>
Message-ID: <2124fb62-2393-8525-4fe9-b95b1b689d09@2ndquadrant.com>
Date: Wed, 29 Mar 2017 01:39:27 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
 Thunderbird/45.7.0
MIME-Version: 1.0
In-Reply-To: <d217ee26-f3d1-4079-0110-3f7ee44ad6b4@2ndquadrant.com>
Content-Type: multipart/mixed;
 boundary="------------2551F435AE34476CFDE570DE"
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

This is a multi-part message in MIME format.
--------------2551F435AE34476CFDE570DE
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit

On 29/03/17 01:29, Petr Jelinek wrote:
> On 28/03/17 18:05, Petr Jelinek wrote:
>> On 28/03/17 17:55, Robert Haas wrote:
>>> On Mon, Mar 27, 2017 at 11:20 PM, Petr Jelinek
>>> <petr.jelinek@2ndquadrant.com> wrote:
>>>> On 28/03/17 04:46, Robert Haas wrote:
>>>>> On Mon, Mar 27, 2017 at 10:04 PM, Andres Freund <andres@anarazel.de> wrote:
>>>>>>> Btw now that I look at the code, I guess we'll want to get rid of
>>>>>>> bgw_main completely in HEAD given that we can't guarantee it will be
>>>>>>> valid even for shared_preload_library libraries. For older branches I
>>>>>>> would leave things as they are in this regard as there don't seem to be
>>>>>>> any immediate issue for standard binaries.
>>>>>>
>>>>>> As long as you fix it so culicidae is happy (in 9.6) ;).  I think it's
>>>>>> fine to just introduce bgw_builtin_id or such, and leave the bgw_main
>>>>>> code in place in < HEAD.
>>>>>
>>>>> I wasn't thinking of introducing bgw_builtin_id.  My idea was just
>>>>> along the lines of
>>>>>
>>>>> if (bgw_library_name == NULL && bgw_function_name != NULL)
>>>>> {
>>>>>     if (strcmp(bgw_function_name, "ParallelQueryMain") == 0)
>>>>>        ParallelQueryMain(blah);
>>>>>     else if (strcmp(bgw_function_name, "LogicalReplicationMain") == 0)
>>>>>        LogicalReplicationMain(blah);
>>>>> }
>>>>>
>>>>> I think something like that is certainly better for the back-branches,
>>>>> because it doesn't cause an ABI break.  But I think it would also be
>>>>> fine for master.
>>>>>
>>>>
>>>> I had something slightly more complex like the attached in mind.
>>>
>>> Seems broadly reasonable on a quick look, but I think we should leave
>>> bgw_main intact in 9.6.  It may be working for fine for people who
>>> don't care about Windows, and I'd rather not break it gratuitously.
>>> Can we have two patches, one of which converts the internal stuff to
>>> use the new mechanism and a second of which removes bgw_main?  The
>>> second one, at least, also needs to update the documentation.  (A good
>>> practice when removing an identifier is to run 'git grep
>>> thing_i_am_removing' after removing it...)
>>>
>>
>> Yes I agree (and I said it in the thread) I plan to submit the 9.6 too,
>> I just sent what I had before having it all ready mainly because you
>> started talking about code already and I didn't want you to waste time
>> with it needlessly, but it was already 5AM for me ;).
>>
> 
> Okay finally got to it, the 9.6 adds similar mechanism as HEAD (it could
> be simpler there but it seems like same code is better) but does not
> remove the bgw_main.
> 

Sigh, forgot git add for the docs, so one more try...

-- 
  Petr Jelinek                  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training & Services

--------------2551F435AE34476CFDE570DE
Content-Type: binary/octet-stream;
 name="Use-library_name-function_name-for-loading-main-entr-96.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename*0="Use-library_name-function_name-for-loading-main-entr-96.patc";
 filename*1="h"

From 9505e5533972b4b027dabcb93644f974bfb4e6ac Mon Sep 17 00:00:00 2001
From: Petr Jelinek <pjmodos@pjmodos.net>
Date: Tue, 28 Mar 2017 23:24:34 +0200
Subject: [PATCH] Use library_name/function_name for loading main entry point
 of internal bgworkers

---
 src/backend/access/transam/parallel.c    |  7 +--
 src/backend/postmaster/bgworker.c        | 76 ++++++++++++++++++++++++++------
 src/include/access/parallel.h            |  2 +
 src/test/modules/worker_spi/worker_spi.c |  6 ++-
 4 files changed, 73 insertions(+), 18 deletions(-)

diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.c
index cde0ed3..d2bed72 100644
--- a/src/backend/access/transam/parallel.c
+++ b/src/backend/access/transam/parallel.c
@@ -109,7 +109,6 @@ static dlist_head pcxt_list = DLIST_STATIC_INIT(pcxt_list);
 /* Private functions. */
 static void HandleParallelMessage(ParallelContext *pcxt, int i, StringInfo msg);
 static void ParallelExtensionTrampoline(dsm_segment *seg, shm_toc *toc);
-static void ParallelWorkerMain(Datum main_arg);
 static void WaitForParallelWorkersToExit(ParallelContext *pcxt);
 
 
@@ -456,7 +455,9 @@ LaunchParallelWorkers(ParallelContext *pcxt)
 		BGWORKER_SHMEM_ACCESS | BGWORKER_BACKEND_DATABASE_CONNECTION;
 	worker.bgw_start_time = BgWorkerStart_ConsistentState;
 	worker.bgw_restart_time = BGW_NEVER_RESTART;
-	worker.bgw_main = ParallelWorkerMain;
+	worker.bgw_main = NULL;
+	sprintf(worker.bgw_library_name, "postgres");
+	sprintf(worker.bgw_function_name, "ParallelWorkerMain");
 	worker.bgw_main_arg = UInt32GetDatum(dsm_segment_handle(pcxt->seg));
 	worker.bgw_notify_pid = MyProcPid;
 	memset(&worker.bgw_extra, 0, BGW_EXTRALEN);
@@ -928,7 +929,7 @@ AtEOXact_Parallel(bool isCommit)
 /*
  * Main entrypoint for parallel workers.
  */
-static void
+void
 ParallelWorkerMain(Datum main_arg)
 {
 	dsm_segment *seg;
diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c
index 52bc4e9..31403bf 100644
--- a/src/backend/postmaster/bgworker.c
+++ b/src/backend/postmaster/bgworker.c
@@ -16,6 +16,7 @@
 
 #include "miscadmin.h"
 #include "libpq/pqsignal.h"
+#include "access/parallel.h"
 #include "postmaster/bgworker_internals.h"
 #include "postmaster/postmaster.h"
 #include "storage/barrier.h"
@@ -94,6 +95,25 @@ struct BackgroundWorkerHandle
 static BackgroundWorkerArray *BackgroundWorkerData;
 
 /*
+ * List of internal background workers. These are used for mapping the
+ * function name to actual function when building with EXEC_BACKEND and also
+ * to allow these to be loaded outside of shared_preload_libraries.
+ */
+typedef struct InternalBGWorkerMain
+{
+	char			   *bgw_function_name;
+	bgworker_main_type	bgw_main;
+} InternalBGWorkerMain;
+
+static const InternalBGWorkerMain InternalBGWorkers[] = {
+	{"ParallelWorkerMain", ParallelWorkerMain},
+	/* Dummy entry marking end of the array. */
+	{NULL, NULL}
+};
+
+static bgworker_main_type GetInternalBgWorkerMain(BackgroundWorker *worker);
+
+/*
  * Calculate shared memory needed.
  */
 Size
@@ -695,22 +715,27 @@ StartBackgroundWorker(void)
 #endif
 	}
 
+	/* For internal workers set the entry point to known function address. */
+	entrypt = GetInternalBgWorkerMain(worker);
+
 	/*
-	 * If bgw_main is set, we use that value as the initial entrypoint.
-	 * However, if the library containing the entrypoint wasn't loaded at
-	 * postmaster startup time, passing it as a direct function pointer is not
-	 * possible.  To work around that, we allow callers for whom a function
-	 * pointer is not available to pass a library name (which will be loaded,
-	 * if necessary) and a function name (which will be looked up in the named
+	 * Otherwise, if bgw_main is set, we use that value as the initial
+	 * entrypoint. This does not work well EXEC_BACKEND outside Windows but
+	 * we keep the logic for backwards compatibility. In other cases use
+	 * the entry point specified by library name (which will be loaded, if
+	 * necessary) and a function name (which will be looked up in the named
 	 * library).
 	 */
-	if (worker->bgw_main != NULL)
-		entrypt = worker->bgw_main;
-	else
-		entrypt = (bgworker_main_type)
-			load_external_function(worker->bgw_library_name,
-								   worker->bgw_function_name,
-								   true, NULL);
+	if (entrypt == NULL)
+	{
+		if (worker->bgw_main != NULL)
+			entrypt = worker->bgw_main;
+		else
+			entrypt = (bgworker_main_type)
+				load_external_function(worker->bgw_library_name,
+									   worker->bgw_function_name,
+									   true, NULL);
+	}
 
 	/*
 	 * Note that in normal processes, we would call InitPostgres here.  For a
@@ -1050,3 +1075,28 @@ TerminateBackgroundWorker(BackgroundWorkerHandle *handle)
 	if (signal_postmaster)
 		SendPostmasterSignal(PMSIGNAL_BACKGROUND_WORKER_CHANGE);
 }
+
+/*
+ * Search the known internal worker array and return its main function
+ * pointer if found.
+ *
+ * Returns NULL if not known internal worker.
+ */
+static bgworker_main_type
+GetInternalBgWorkerMain(BackgroundWorker *worker)
+{
+	int i;
+
+	/* Internal workers always have to use postgres as library name. */
+	if (strncmp(worker->bgw_library_name, "postgres", 8) != 0)
+		return NULL;
+
+	for (i = 0; InternalBGWorkers[i].bgw_function_name; i++)
+	{
+		if (strncmp(InternalBGWorkers[i].bgw_function_name,
+					worker->bgw_function_name, BGW_MAXLEN) == 0)
+			return InternalBGWorkers[i].bgw_main;
+	}
+
+	return NULL;
+}
diff --git a/src/include/access/parallel.h b/src/include/access/parallel.h
index 2f8f36f..61dbfda 100644
--- a/src/include/access/parallel.h
+++ b/src/include/access/parallel.h
@@ -67,4 +67,6 @@ extern void AtEOXact_Parallel(bool isCommit);
 extern void AtEOSubXact_Parallel(bool isCommit, SubTransactionId mySubId);
 extern void ParallelWorkerReportLastRecEnd(XLogRecPtr last_xlog_end);
 
+extern void ParallelWorkerMain(Datum main_arg);
+
 #endif   /* PARALLEL_H */
diff --git a/src/test/modules/worker_spi/worker_spi.c b/src/test/modules/worker_spi/worker_spi.c
index 7c9a3eb..3711fba 100644
--- a/src/test/modules/worker_spi/worker_spi.c
+++ b/src/test/modules/worker_spi/worker_spi.c
@@ -346,7 +346,9 @@ _PG_init(void)
 		BGWORKER_BACKEND_DATABASE_CONNECTION;
 	worker.bgw_start_time = BgWorkerStart_RecoveryFinished;
 	worker.bgw_restart_time = BGW_NEVER_RESTART;
-	worker.bgw_main = worker_spi_main;
+	worker.bgw_main = NULL;
+	sprintf(worker.bgw_library_name, "worker_spi");
+	sprintf(worker.bgw_function_name, "worker_spi_main");
 	worker.bgw_notify_pid = 0;
 
 	/*
@@ -377,7 +379,7 @@ worker_spi_launch(PG_FUNCTION_ARGS)
 		BGWORKER_BACKEND_DATABASE_CONNECTION;
 	worker.bgw_start_time = BgWorkerStart_RecoveryFinished;
 	worker.bgw_restart_time = BGW_NEVER_RESTART;
-	worker.bgw_main = NULL;		/* new worker might not have library loaded */
+	worker.bgw_main = NULL;
 	sprintf(worker.bgw_library_name, "worker_spi");
 	sprintf(worker.bgw_function_name, "worker_spi_main");
 	snprintf(worker.bgw_name, BGW_MAXLEN, "worker %d", i);
-- 
2.7.4


--------------2551F435AE34476CFDE570DE
Content-Type: binary/octet-stream;
 name="Use-library_name-function_name-for-loading-main-entr-HEAD.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename*0="Use-library_name-function_name-for-loading-main-entr-HEAD.pa";
 filename*1="tch"

From 07f53d1ff4abdd7e1a7876f6dd8e324c83eb2610 Mon Sep 17 00:00:00 2001
From: Petr Jelinek <pjmodos@pjmodos.net>
Date: Tue, 28 Mar 2017 04:21:46 +0200
Subject: [PATCH] Use library_name/function_name for loading main entry point
 of internal bgworkers

Also remove bgw_main as it's not safe to use even for
shared_preload_libraries.
---
 doc/src/sgml/bgworker.sgml                 | 36 ++++---------
 src/backend/access/transam/parallel.c      |  6 +--
 src/backend/postmaster/bgworker.c          | 81 +++++++++++++++++++-----------
 src/backend/replication/logical/launcher.c |  6 ++-
 src/include/access/parallel.h              |  2 +
 src/include/postmaster/bgworker.h          |  5 +-
 src/test/modules/test_shm_mq/setup.c       |  1 -
 src/test/modules/worker_spi/worker_spi.c   |  4 +-
 8 files changed, 75 insertions(+), 66 deletions(-)

diff --git a/doc/src/sgml/bgworker.sgml b/doc/src/sgml/bgworker.sgml
index 07f9f10..33cfdaf 100644
--- a/doc/src/sgml/bgworker.sgml
+++ b/doc/src/sgml/bgworker.sgml
@@ -54,9 +54,8 @@ typedef struct BackgroundWorker
     int         bgw_flags;
     BgWorkerStartTime bgw_start_time;
     int         bgw_restart_time;       /* in seconds, or BGW_NEVER_RESTART */
-    bgworker_main_type bgw_main;
-    char        bgw_library_name[BGW_MAXLEN];   /* only if bgw_main is NULL */
-    char        bgw_function_name[BGW_MAXLEN];  /* only if bgw_main is NULL */
+    char        bgw_library_name[BGW_MAXLEN];
+    char        bgw_function_name[BGW_MAXLEN];
     Datum       bgw_main_arg;
     char        bgw_extra[BGW_EXTRALEN];
     int         bgw_notify_pid;
@@ -131,26 +130,12 @@ typedef struct BackgroundWorker
   </para>
 
   <para>
-   <structfield>bgw_main</structfield> is a pointer to the function to run when
-   the process is started.  This field can only safely be used to launch
-   functions within the core server, because shared libraries may be loaded
-   at different starting addresses in different backend processes.  This will
-   happen on all platforms when the library is loaded using any mechanism
-   other than <xref linkend="guc-shared-preload-libraries">.  Even when that
-   mechanism is used, address space layout variations will still occur on
-   Windows, and when <literal>EXEC_BACKEND</> is used.  Therefore, most users
-   of this API should set this field to NULL.  If it is non-NULL, it takes
-   precedence over <structfield>bgw_library_name</> and
-   <structfield>bgw_function_name</>.
-  </para>
-
-  <para>
    <structfield>bgw_library_name</structfield> is the name of a library in
    which the initial entry point for the background worker should be sought.
    The named library will be dynamically loaded by the worker process and
    <structfield>bgw_function_name</structfield> will be used to identify the
-   function to be called.  If loading a function from the core code,
-   <structfield>bgw_main</> should be set instead.
+   function to be called.  If loading a function from the core code, this must
+   be set to "postgres".
   </para>
 
   <para>
@@ -161,13 +146,10 @@ typedef struct BackgroundWorker
 
   <para>
    <structfield>bgw_main_arg</structfield> is the <type>Datum</> argument
-   to the background worker main function.  Regardless of whether that
-   function is specified via <structfield>bgw_main</> or via the combination
-   of <function>bgw_library_name</> and <function>bgw_function_name</>,
-   this main function should take a single argument of type <type>Datum</>
-   and return <type>void</>.  <structfield>bgw_main_arg</structfield> will be
-   passed as the argument.  In addition, the global variable
-   <literal>MyBgworkerEntry</literal>
+   to the background worker main function.  This main function should take a
+   single argument of type <type>Datum</> and return <type>void</>.
+   <structfield>bgw_main_arg</structfield> will be passed as the argument.
+   In addition, the global variable <literal>MyBgworkerEntry</literal>
    points to a copy of the <structname>BackgroundWorker</structname> structure
    passed at registration time; the worker may find it helpful to examine
    this structure.
@@ -215,7 +197,7 @@ typedef struct BackgroundWorker
 
   <para>
    Signals are initially blocked when control reaches the
-   <structfield>bgw_main</> function, and must be unblocked by it; this is to
+   background workers' main function, and must be unblocked by it; this is to
    allow the process to customize its signal handlers, if necessary.
    Signals can be unblocked in the new process by calling
    <function>BackgroundWorkerUnblockSignals</> and blocked by calling
diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.c
index 3e0ee87..b3d3853 100644
--- a/src/backend/access/transam/parallel.c
+++ b/src/backend/access/transam/parallel.c
@@ -110,7 +110,6 @@ static dlist_head pcxt_list = DLIST_STATIC_INIT(pcxt_list);
 /* Private functions. */
 static void HandleParallelMessage(ParallelContext *pcxt, int i, StringInfo msg);
 static void ParallelExtensionTrampoline(dsm_segment *seg, shm_toc *toc);
-static void ParallelWorkerMain(Datum main_arg);
 static void WaitForParallelWorkersToExit(ParallelContext *pcxt);
 
 
@@ -458,7 +457,8 @@ LaunchParallelWorkers(ParallelContext *pcxt)
 		| BGWORKER_CLASS_PARALLEL;
 	worker.bgw_start_time = BgWorkerStart_ConsistentState;
 	worker.bgw_restart_time = BGW_NEVER_RESTART;
-	worker.bgw_main = ParallelWorkerMain;
+	sprintf(worker.bgw_library_name, "postgres");
+	sprintf(worker.bgw_function_name, "ParallelWorkerMain");
 	worker.bgw_main_arg = UInt32GetDatum(dsm_segment_handle(pcxt->seg));
 	worker.bgw_notify_pid = MyProcPid;
 	memset(&worker.bgw_extra, 0, BGW_EXTRALEN);
@@ -931,7 +931,7 @@ AtEOXact_Parallel(bool isCommit)
 /*
  * Main entrypoint for parallel workers.
  */
-static void
+void
 ParallelWorkerMain(Datum main_arg)
 {
 	dsm_segment *seg;
diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c
index 10e0f88..39a098c 100644
--- a/src/backend/postmaster/bgworker.c
+++ b/src/backend/postmaster/bgworker.c
@@ -15,12 +15,14 @@
 #include <unistd.h>
 
 #include "libpq/pqsignal.h"
+#include "access/parallel.h"
 #include "miscadmin.h"
 #include "pgstat.h"
 #include "port/atomics.h"
 #include "postmaster/bgworker_internals.h"
 #include "postmaster/postmaster.h"
 #include "replication/logicallauncher.h"
+#include "replication/logicalworker.h"
 #include "storage/dsm.h"
 #include "storage/ipc.h"
 #include "storage/latch.h"
@@ -109,14 +111,26 @@ struct BackgroundWorkerHandle
 static BackgroundWorkerArray *BackgroundWorkerData;
 
 /*
- * List of workers that are allowed to be started outside of
- * shared_preload_libraries.
+ * List of internal background workers. These are used for mapping the
+ * function name to actual function when building with EXEC_BACKEND and also
+ * to allow these to be loaded outside of shared_preload_libraries.
  */
-static const bgworker_main_type InternalBGWorkers[] = {
-	ApplyLauncherMain,
-	NULL
+typedef struct InternalBGWorkerMain
+{
+	char			   *bgw_function_name;
+	bgworker_main_type	bgw_main;
+} InternalBGWorkerMain;
+
+static const InternalBGWorkerMain InternalBGWorkers[] = {
+	{"ParallelWorkerMain", ParallelWorkerMain},
+	{"ApplyLauncherMain", ApplyLauncherMain},
+	{"ApplyWorkerMain", ApplyWorkerMain},
+	/* Dummy entry marking end of the array. */
+	{NULL, NULL}
 };
 
+static bgworker_main_type GetInternalBgWorkerMain(BackgroundWorker *worker);
+
 /*
  * Calculate shared memory needed.
  */
@@ -341,7 +355,6 @@ BackgroundWorkerStateChange(void)
 		rw->rw_worker.bgw_flags = slot->worker.bgw_flags;
 		rw->rw_worker.bgw_start_time = slot->worker.bgw_start_time;
 		rw->rw_worker.bgw_restart_time = slot->worker.bgw_restart_time;
-		rw->rw_worker.bgw_main = slot->worker.bgw_main;
 		rw->rw_worker.bgw_main_arg = slot->worker.bgw_main_arg;
 		memcpy(rw->rw_worker.bgw_extra, slot->worker.bgw_extra, BGW_EXTRALEN);
 
@@ -763,17 +776,14 @@ StartBackgroundWorker(void)
 	}
 
 	/*
-	 * If bgw_main is set, we use that value as the initial entrypoint.
-	 * However, if the library containing the entrypoint wasn't loaded at
-	 * postmaster startup time, passing it as a direct function pointer is not
-	 * possible.  To work around that, we allow callers for whom a function
-	 * pointer is not available to pass a library name (which will be loaded,
-	 * if necessary) and a function name (which will be looked up in the named
-	 * library).
+	 * For internal workers set the entry point to known function address.
+	 * Otherwise use the entry point specified by library name (which will
+	 * be loaded, if necessary) and a function name (which will be looked up
+	 * in the named library).
 	 */
-	if (worker->bgw_main != NULL)
-		entrypt = worker->bgw_main;
-	else
+	entrypt = GetInternalBgWorkerMain(worker);
+
+	if (entrypt == NULL)
 		entrypt = (bgworker_main_type)
 			load_external_function(worker->bgw_library_name,
 								   worker->bgw_function_name,
@@ -806,23 +816,13 @@ RegisterBackgroundWorker(BackgroundWorker *worker)
 {
 	RegisteredBgWorker *rw;
 	static int	numworkers = 0;
-	bool		internal = false;
-	int			i;
 
 	if (!IsUnderPostmaster)
 		ereport(DEBUG1,
 		 (errmsg("registering background worker \"%s\"", worker->bgw_name)));
 
-	for (i = 0; InternalBGWorkers[i]; i++)
-	{
-		if (worker->bgw_main == InternalBGWorkers[i])
-		{
-			internal = true;
-			break;
-		}
-	}
-
-	if (!process_shared_preload_libraries_in_progress && !internal)
+	if (!process_shared_preload_libraries_in_progress &&
+		GetInternalBgWorkerMain(worker) == NULL)
 	{
 		if (!IsUnderPostmaster)
 			ereport(LOG,
@@ -1152,3 +1152,28 @@ TerminateBackgroundWorker(BackgroundWorkerHandle *handle)
 	if (signal_postmaster)
 		SendPostmasterSignal(PMSIGNAL_BACKGROUND_WORKER_CHANGE);
 }
+
+/*
+ * Search the known internal worker array and return its main function
+ * pointer if found.
+ *
+ * Returns NULL if not known internal worker.
+ */
+static bgworker_main_type
+GetInternalBgWorkerMain(BackgroundWorker *worker)
+{
+	int i;
+
+	/* Internal workers always have to use postgres as library name. */
+	if (strncmp(worker->bgw_library_name, "postgres", 8) != 0)
+		return NULL;
+
+	for (i = 0; InternalBGWorkers[i].bgw_function_name; i++)
+	{
+		if (strncmp(InternalBGWorkers[i].bgw_function_name,
+					worker->bgw_function_name, BGW_MAXLEN) == 0)
+			return InternalBGWorkers[i].bgw_main;
+	}
+
+	return NULL;
+}
diff --git a/src/backend/replication/logical/launcher.c b/src/backend/replication/logical/launcher.c
index 0d3ec27..226a168 100644
--- a/src/backend/replication/logical/launcher.c
+++ b/src/backend/replication/logical/launcher.c
@@ -295,7 +295,8 @@ logicalrep_worker_launch(Oid dbid, Oid subid, const char *subname, Oid userid,
 	bgw.bgw_flags =	BGWORKER_SHMEM_ACCESS |
 		BGWORKER_BACKEND_DATABASE_CONNECTION;
 	bgw.bgw_start_time = BgWorkerStart_RecoveryFinished;
-	bgw.bgw_main = ApplyWorkerMain;
+	snprintf(bgw.bgw_library_name, BGW_MAXLEN, "postgres");
+	snprintf(bgw.bgw_function_name, BGW_MAXLEN, "ApplyWorkerMain");
 	if (OidIsValid(relid))
 		snprintf(bgw.bgw_name, BGW_MAXLEN,
 				 "logical replication worker for subscription %u sync %u", subid, relid);
@@ -553,7 +554,8 @@ ApplyLauncherRegister(void)
 	bgw.bgw_flags =	BGWORKER_SHMEM_ACCESS |
 		BGWORKER_BACKEND_DATABASE_CONNECTION;
 	bgw.bgw_start_time = BgWorkerStart_RecoveryFinished;
-	bgw.bgw_main = ApplyLauncherMain;
+	snprintf(bgw.bgw_library_name, BGW_MAXLEN, "postgres");
+	snprintf(bgw.bgw_function_name, BGW_MAXLEN, "ApplyLauncherMain");
 	snprintf(bgw.bgw_name, BGW_MAXLEN,
 			 "logical replication launcher");
 	bgw.bgw_restart_time = 5;
diff --git a/src/include/access/parallel.h b/src/include/access/parallel.h
index 96ce480..5065a38 100644
--- a/src/include/access/parallel.h
+++ b/src/include/access/parallel.h
@@ -67,4 +67,6 @@ extern void AtEOXact_Parallel(bool isCommit);
 extern void AtEOSubXact_Parallel(bool isCommit, SubTransactionId mySubId);
 extern void ParallelWorkerReportLastRecEnd(XLogRecPtr last_xlog_end);
 
+extern void ParallelWorkerMain(Datum main_arg);
+
 #endif   /* PARALLEL_H */
diff --git a/src/include/postmaster/bgworker.h b/src/include/postmaster/bgworker.h
index 128e92c..51a5978 100644
--- a/src/include/postmaster/bgworker.h
+++ b/src/include/postmaster/bgworker.h
@@ -91,9 +91,8 @@ typedef struct BackgroundWorker
 	int			bgw_flags;
 	BgWorkerStartTime bgw_start_time;
 	int			bgw_restart_time;		/* in seconds, or BGW_NEVER_RESTART */
-	bgworker_main_type bgw_main;
-	char		bgw_library_name[BGW_MAXLEN];	/* only if bgw_main is NULL */
-	char		bgw_function_name[BGW_MAXLEN];	/* only if bgw_main is NULL */
+	char		bgw_library_name[BGW_MAXLEN];
+	char		bgw_function_name[BGW_MAXLEN];
 	Datum		bgw_main_arg;
 	char		bgw_extra[BGW_EXTRALEN];
 	pid_t		bgw_notify_pid; /* SIGUSR1 this backend on start/stop */
diff --git a/src/test/modules/test_shm_mq/setup.c b/src/test/modules/test_shm_mq/setup.c
index e3c024e..319a67f 100644
--- a/src/test/modules/test_shm_mq/setup.c
+++ b/src/test/modules/test_shm_mq/setup.c
@@ -216,7 +216,6 @@ setup_background_workers(int nworkers, dsm_segment *seg)
 	worker.bgw_flags = BGWORKER_SHMEM_ACCESS;
 	worker.bgw_start_time = BgWorkerStart_ConsistentState;
 	worker.bgw_restart_time = BGW_NEVER_RESTART;
-	worker.bgw_main = NULL;		/* new worker might not have library loaded */
 	sprintf(worker.bgw_library_name, "test_shm_mq");
 	sprintf(worker.bgw_function_name, "test_shm_mq_main");
 	snprintf(worker.bgw_name, BGW_MAXLEN, "test_shm_mq");
diff --git a/src/test/modules/worker_spi/worker_spi.c b/src/test/modules/worker_spi/worker_spi.c
index 72ab846..421ec76 100644
--- a/src/test/modules/worker_spi/worker_spi.c
+++ b/src/test/modules/worker_spi/worker_spi.c
@@ -347,7 +347,8 @@ _PG_init(void)
 		BGWORKER_BACKEND_DATABASE_CONNECTION;
 	worker.bgw_start_time = BgWorkerStart_RecoveryFinished;
 	worker.bgw_restart_time = BGW_NEVER_RESTART;
-	worker.bgw_main = worker_spi_main;
+	sprintf(worker.bgw_library_name, "worker_spi");
+	sprintf(worker.bgw_function_name, "worker_spi_main");
 	worker.bgw_notify_pid = 0;
 
 	/*
@@ -378,7 +379,6 @@ worker_spi_launch(PG_FUNCTION_ARGS)
 		BGWORKER_BACKEND_DATABASE_CONNECTION;
 	worker.bgw_start_time = BgWorkerStart_RecoveryFinished;
 	worker.bgw_restart_time = BGW_NEVER_RESTART;
-	worker.bgw_main = NULL;		/* new worker might not have library loaded */
 	sprintf(worker.bgw_library_name, "worker_spi");
 	sprintf(worker.bgw_function_name, "worker_spi_main");
 	snprintf(worker.bgw_name, BGW_MAXLEN, "worker %d", i);
-- 
2.7.4


--------------2551F435AE34476CFDE570DE
Content-Type: text/plain
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

--------------2551F435AE34476CFDE570DE--