Don't use bgw_main even to specify in-core bgworker entrypoints.

On EXEC_BACKEND builds, this can fail if ASLR is in use.

Backpatch to 9.5.  On master, completely remove the bgw_main field
completely, since there is no situation in which it is safe for an
EXEC_BACKEND build.  On 9.6 and 9.5, leave the field intact to avoid
breaking things for third-party code that doesn't care about working
under EXEC_BACKEND.  Prior to 9.5, there are no in-core bgworker
entrypoints.

Petr Jelinek, reviewed by me.

Discussion: http://postgr.es/m/09d8ad33-4287-a09b-a77f-77f8761adb5e@2ndquadrant.com

diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.c
index cde0ed300f7130c396640d8c496519ff191be432..d2bed7270571d19bf73b8ecfbb5f8de0e3f651b8 100644 (file)
--- 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 52bc4e962fb0d3d2c1e9f7687a8aafd8b0d4f2d5..5ea5abff8da20aba82d1972708d804dcf054e767 100644 (file)
--- 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"
@@ -93,6 +94,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.
  */
@@ -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", BGW_MAXLEN) != 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 2f8f36fea4a199983a39638fde3a8228ae00992e..61dbfda478883b9e6c9aa1a08eda2dd85aaebf5a 100644 (file)
--- 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 7c9a3eb67e3427f6f291d84cbbfb76d8091128e1..3711fba6fc9d70b994e24b4108d8dcef51ebf25e 100644 (file)
--- 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);