Refactor pid, random seed and start time initialization.

Background workers, including parallel workers, were generating
the same sequence of numbers in random().  This showed up as DSM
handle collisions when Parallel Hash created multiple segments,
but any code that calls random() in background workers could be
affected if it cares about different backends generating different
numbers.

Repair by making sure that all new processes initialize the seed
at the same time as they set MyProcPid and MyStartTime in a new
function InitProcessGlobals(), called by the postmaster, its
children and also standalone processes.  Also add a new high
resolution MyStartTimestamp as a potentially useful by-product,
and remove SessionStartTime from struct Port as it is now
redundant.

No back-patch for now, as the known consequences so far are just
a bunch of harmless shm_open(O_EXCL) collisions.

Author: Thomas Munro
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/CAEepm%3D2eJj_6%3DB%2B2tEpGu2nf1BjthCf9nXXUouYvJJ4C5WSwhg%40mail.gmail.com

diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 8a5b2b3b420b72e242f246409919ffea89af1148..8de603d19331665b2ddb3b49046255259a2f7deb 100644 (file)
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -2784,21 +2784,13 @@ pgstat_initialize(void)
 void
 pgstat_bestart(void)
 {
-       TimestampTz proc_start_timestamp;
        SockAddr        clientaddr;
        volatile PgBackendStatus *beentry;
 
        /*
         * To minimize the time spent modifying the PgBackendStatus entry, fetch
         * all the needed data first.
-        *
-        * If we have a MyProcPort, use its session start time (for consistency,
-        * and to save a kernel call).
         */
-       if (MyProcPort)
-               proc_start_timestamp = MyProcPort->SessionStartTime;
-       else
-               proc_start_timestamp = GetCurrentTimestamp();
 
        /*
         * We may not have a MyProcPort (eg, if this is the autovacuum process).
@@ -2883,7 +2875,7 @@ pgstat_bestart(void)
        } while ((beentry->st_changecount & 1) == 0);
 
        beentry->st_procpid = MyProcPid;
-       beentry->st_proc_start_timestamp = proc_start_timestamp;
+       beentry->st_proc_start_timestamp = MyStartTimestamp;
        beentry->st_activity_start_timestamp = 0;
        beentry->st_state_start_timestamp = 0;
        beentry->st_xact_start_timestamp = 0;

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 41de140ae013f228ed876879597932a388c5b62c..688f462e7d0eb363a42075e87693f3ea43073bdb 100644 (file)
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -129,6 +129,7 @@
 #include "utils/pidfile.h"
 #include "utils/ps_status.h"
 #include "utils/timeout.h"
+#include "utils/timestamp.h"
 #include "utils/varlena.h"
 
 #ifdef EXEC_BACKEND
@@ -581,9 +582,9 @@ PostmasterMain(int argc, char *argv[])
        int                     i;
        char       *output_config_variable = NULL;
 
-       MyProcPid = PostmasterPid = getpid();
+       InitProcessGlobals();
 
-       MyStartTime = time(NULL);
+       PostmasterPid = MyProcPid;
 
        IsPostmasterEnvironment = true;
 
@@ -597,16 +598,6 @@ PostmasterMain(int argc, char *argv[])
         */
        umask(PG_MODE_MASK_OWNER);
 
-       /*
-        * Initialize random(3) so we don't get the same values in every run.
-        *
-        * Note: the seed is pretty predictable from externally-visible facts such
-        * as postmaster start time, so avoid using random() for security-critical
-        * random values during postmaster startup.  At the time of first
-        * connection, PostmasterRandom will select a hopefully-more-random seed.
-        */
-       srandom((unsigned int) (MyProcPid ^ MyStartTime));
-
        /*
         * By default, palloc() requests in the postmaster will be allocated in
         * the PostmasterContext, which is space that can be recycled by backends.
@@ -2513,6 +2504,32 @@ ClosePostmasterPorts(bool am_syslogger)
 }
 
 
+/*
+ * InitProcessGlobals -- set MyProcPid, MyStartTime[stamp], random seeds
+ *
+ * Called early in every backend.
+ */
+void
+InitProcessGlobals(void)
+{
+       MyProcPid = getpid();
+       MyStartTimestamp = GetCurrentTimestamp();
+       MyStartTime = timestamptz_to_time_t(MyStartTimestamp);
+
+       /*
+        * Don't want backend to be able to see the postmaster random number
+        * generator state.  We have to clobber the static random_seed.
+        */
+#ifndef HAVE_STRONG_RANDOM
+       random_seed = 0;
+       random_start_time.tv_usec = 0;
+#endif
+
+       /* Set a different seed for random() in every backend. */
+       srandom((unsigned int) MyProcPid ^ (unsigned int) MyStartTimestamp);
+}
+
+
 /*
  * reset_shared -- reset shared memory and semaphores
  */
@@ -4154,10 +4171,6 @@ BackendInitialize(Port *port)
        /* This flag will remain set until InitPostgres finishes authentication */
        ClientAuthInProgress = true;    /* limit visibility of log messages */
 
-       /* save process start time */
-       port->SessionStartTime = GetCurrentTimestamp();
-       MyStartTime = timestamptz_to_time_t(port->SessionStartTime);
-
        /* set these to empty in case they are needed before we set them up */
        port->remote_host = "";
        port->remote_port = "";
@@ -4315,23 +4328,8 @@ BackendRun(Port *port)
        char      **av;
        int                     maxac;
        int                     ac;
-       long            secs;
-       int                     usecs;
        int                     i;
 
-       /*
-        * Don't want backend to be able to see the postmaster random number
-        * generator state.  We have to clobber the static random_seed *and* start
-        * a new random sequence in the random() library function.
-        */
-#ifndef HAVE_STRONG_RANDOM
-       random_seed = 0;
-       random_start_time.tv_usec = 0;
-#endif
-       /* slightly hacky way to convert timestamptz into integers */
-       TimestampDifference(0, port->SessionStartTime, &secs, &usecs);
-       srandom((unsigned int) (MyProcPid ^ (usecs << 12) ^ secs));
-
        /*
         * Now, build the argv vector that will be given to PostgresMain.
         *

diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index e4c6e3d406e9928158feebf1fbd7fb9445db9afc..6e13d14fcd0e9c707a8eda897a46566e7aac05e6 100644 (file)
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -4632,7 +4632,7 @@ log_disconnections(int code, Datum arg)
                                minutes,
                                seconds;
 
-       TimestampDifference(port->SessionStartTime,
+       TimestampDifference(MyStartTimestamp,
                                                GetCurrentTimestamp(),
                                                &secs, &usecs);
        msecs = usecs / 1000;

diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c
index 5971310aabb084507e780487b7278e00c3cc8608..c6939779b98efb1e82c69e7358cbaf88a6f8584d 100644 (file)
--- a/src/backend/utils/init/globals.c
+++ b/src/backend/utils/init/globals.c
@@ -39,6 +39,7 @@ volatile uint32 CritSectionCount = 0;
 
 int                    MyProcPid;
 pg_time_t      MyStartTime;
+TimestampTz    MyStartTimestamp;
 struct Port *MyProcPort;
 int32          MyCancelKey;
 int                    MyPMChildSlot;

diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index f590ecb25e87f0e2c1813148fe1cee37ccea71b7..238fe1deec85274d8392296b963ccdf5ca19c1a8 100644 (file)
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -274,9 +274,7 @@ InitPostmasterChild(void)
 {
        IsUnderPostmaster = true;       /* we are a postmaster subprocess now */
 
-       MyProcPid = getpid();           /* reset MyProcPid */
-
-       MyStartTime = time(NULL);       /* set our start time in case we call elog */
+       InitProcessGlobals();
 
        /*
         * make sure stderr is in binary mode before anything can possibly be
@@ -321,17 +319,7 @@ InitStandaloneProcess(const char *argv0)
 {
        Assert(!IsPostmasterEnvironment);
 
-       MyProcPid = getpid();           /* reset MyProcPid */
-
-       MyStartTime = time(NULL);       /* set our start time in case we call elog */
-
-       /*
-        * Initialize random() for the first time, like PostmasterMain() would.
-        * In a regular IsUnderPostmaster backend, BackendRun() computes a
-        * high-entropy seed before any user query.  Fewer distinct initial seeds
-        * can occur here.
-        */
-       srandom((unsigned int) (MyProcPid ^ MyStartTime));
+       InitProcessGlobals();
 
        /* Initialize process-local latch support */
        InitializeLatchSupport();

diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h
index eb8bba4ed8830acb9d9350a693cae991ce1bd596..b2c59df54e4c42e03d4333248b5c901b1c28b36f 100644 (file)
--- a/src/include/libpq/libpq-be.h
+++ b/src/include/libpq/libpq-be.h
@@ -150,13 +150,6 @@ typedef struct Port
         */
        HbaLine    *hba;
 
-       /*
-        * Information that really has no business at all being in struct Port,
-        * but since it gets used by elog.c in the same way as database_name and
-        * other members of this struct, we may as well keep it here.
-        */
-       TimestampTz SessionStartTime;   /* backend start time */
-
        /*
         * TCP keepalive settings.
         *

diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 69f356f8cdb22efc32e430e87bb4b1202f5325e3..d6b32c070c232f031cf70ea8c0f00c1615c02738 100644 (file)
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -25,6 +25,7 @@
 
 #include <signal.h>
 
+#include "datatype/timestamp.h"        /* for TimestampTZ */
 #include "pgtime.h"                            /* for pg_time_t */
 
 
@@ -163,6 +164,7 @@ extern PGDLLIMPORT int max_parallel_workers;
 
 extern PGDLLIMPORT int MyProcPid;
 extern PGDLLIMPORT pg_time_t MyStartTime;
+extern PGDLLIMPORT TimestampTz MyStartTimestamp;
 extern PGDLLIMPORT struct Port *MyProcPort;
 extern PGDLLIMPORT struct Latch *MyLatch;
 extern int32 MyCancelKey;

diff --git a/src/include/postmaster/postmaster.h b/src/include/postmaster/postmaster.h
index 1877eef2391639e39e4d99b95e7d56214b7fe3b6..a40d66e89065fea9ce3bb581363c12927ce2fcd7 100644 (file)
--- a/src/include/postmaster/postmaster.h
+++ b/src/include/postmaster/postmaster.h
@@ -48,6 +48,7 @@ extern PGDLLIMPORT const char *progname;
 
 extern void PostmasterMain(int argc, char *argv[]) pg_attribute_noreturn();
 extern void ClosePostmasterPorts(bool am_syslogger);
+extern void InitProcessGlobals(void);
 
 extern int     MaxLivePostmasterChildren(void);