Revert "Use pselect(2) not select(2), if available, to wait in postmaster's loop."

This reverts commit b9515b62879722e3497236f6e0d6783c3fc059a2.

Buildfarm results suggest that some platforms have versions of pselect(2)
that are not merely non-atomic, but flat out non-functional.  Revert the
use-pselect patch to confirm this diagnosis (and exclude the no-SA_RESTART
patch as the source of trouble).  If it's so, we should probably look into
blacklisting specific platforms that have broken pselect.

Discussion: https://postgr.es/m/9696.1493072081@sss.pgh.pa.us

diff --git a/configure b/configure
index 21631258f6b5d51383d9d9baedc6d4f7ce53d666..bb285e4e1b3c01e09a1e054f24e00a1024ad81cd 100755 (executable)
--- a/configure
+++ b/configure
@@ -12504,7 +12504,7 @@ fi
 LIBS_including_readline="$LIBS"
 LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
 
-for ac_func in cbrt dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll pselect pstat pthread_is_threaded_np readlink setproctitle setsid shm_open symlink sync_file_range towlower utime utimes wcstombs wcstombs_l
+for ac_func in cbrt dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll pstat pthread_is_threaded_np readlink setproctitle setsid shm_open symlink sync_file_range towlower utime utimes wcstombs wcstombs_l
 do :
   as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
 ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"

diff --git a/configure.in b/configure.in
index 319ad43ab9f8eb00c8020c5b7d36e31f364b0f54..09a887d4f0bafb0a1f0c160de8006f1d6602b723 100644 (file)
--- a/configure.in
+++ b/configure.in
@@ -1456,7 +1456,7 @@ PGAC_FUNC_WCSTOMBS_L
 LIBS_including_readline="$LIBS"
 LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
 
-AC_CHECK_FUNCS([cbrt dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll pselect pstat pthread_is_threaded_np readlink setproctitle setsid shm_open symlink sync_file_range towlower utime utimes wcstombs wcstombs_l])
+AC_CHECK_FUNCS([cbrt dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll pstat pthread_is_threaded_np readlink setproctitle setsid shm_open symlink sync_file_range towlower utime utimes wcstombs wcstombs_l])
 
 AC_REPLACE_FUNCS(fseeko)
 case $host_os in

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index cd793886f834218414a4c8dbb4be148ebb411e73..5a60995889630c0fbbe733db49ecfa0f9af1ae36 100644 (file)
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -606,9 +606,9 @@ PostmasterMain(int argc, char *argv[])
     * In the postmaster, we want to install non-ignored handlers *without*
     * SA_RESTART.  This is because they'll be blocked at all times except
     * when ServerLoop is waiting for something to happen, and during that
-    * window, we want signals to exit the pselect(2) wait so that ServerLoop
+    * window, we want signals to exit the select(2) wait so that ServerLoop
     * can respond if anything interesting happened.  On some platforms,
-    * signals marked SA_RESTART would not cause the pselect() wait to end.
+    * signals marked SA_RESTART would not cause the select() wait to end.
     * Child processes will generally want SA_RESTART, but we expect them to
     * set up their own handlers before unblocking signals.
     *
@@ -1642,8 +1642,6 @@ ServerLoop(void)
    for (;;)
    {
        fd_set      rmask;
-       fd_set     *rmask_p;
-       struct timeval timeout;
        int         selres;
        time_t      now;
 
@@ -1653,64 +1651,37 @@ ServerLoop(void)
         * We block all signals except while sleeping. That makes it safe for
         * signal handlers, which again block all signals while executing, to
         * do nontrivial work.
+        *
+        * If we are in PM_WAIT_DEAD_END state, then we don't want to accept
+        * any new connections, so we don't call select(), and just sleep.
         */
+       memcpy((char *) &rmask, (char *) &readmask, sizeof(fd_set));
+
        if (pmState == PM_WAIT_DEAD_END)
        {
-           /*
-            * If we are in PM_WAIT_DEAD_END state, then we don't want to
-            * accept any new connections, so pass a null rmask.
-            */
-           rmask_p = NULL;
-           timeout.tv_sec = 0;
-           timeout.tv_usec = 100000;   /* 100 msec seems reasonable */
+           PG_SETMASK(&UnBlockSig);
+
+           pg_usleep(100000L); /* 100 msec seems reasonable */
+           selres = 0;
+
+           PG_SETMASK(&BlockSig);
        }
        else
        {
-           /* Normal case: check sockets, and compute a suitable timeout */
-           memcpy(&rmask, &readmask, sizeof(fd_set));
-           rmask_p = &rmask;
+           /* must set timeout each time; some OSes change it! */
+           struct timeval timeout;
 
            /* Needs to run with blocked signals! */
            DetermineSleepTime(&timeout);
-       }
 
-       /*
-        * We prefer to wait with pselect(2) if available, as using that,
-        * together with *not* using SA_RESTART for signals, guarantees that
-        * we will get kicked off the wait if a signal occurs.
-        *
-        * If we lack pselect(2), fake it with select(2).  This has a race
-        * condition: a signal that was already pending will be delivered
-        * before we reach the select(), and therefore the select() will wait,
-        * even though we might wish to do something in response.  Therefore,
-        * beware of putting any time-critical signal response logic into
-        * ServerLoop rather than into the signal handler itself.  It will run
-        * eventually, but maybe not till after a timeout delay.
-        *
-        * Some implementations of pselect() are reportedly not atomic, making
-        * the first alternative here functionally equivalent to the second.
-        * Not much we can do about that though.
-        */
-       {
-#ifdef HAVE_PSELECT
-           /* pselect uses a randomly different timeout API, sigh */
-           struct timespec ptimeout;
-
-           ptimeout.tv_sec = timeout.tv_sec;
-           ptimeout.tv_nsec = timeout.tv_usec * 1000;
-
-           selres = pselect(nSockets, rmask_p, NULL, NULL,
-                            &ptimeout, &UnBlockSig);
-#else
            PG_SETMASK(&UnBlockSig);
 
-           selres = select(nSockets, rmask_p, NULL, NULL, &timeout);
+           selres = select(nSockets, &rmask, NULL, NULL, &timeout);
 
            PG_SETMASK(&BlockSig);
-#endif
        }
 
-       /* Now check the select()/pselect() result */
+       /* Now check the select() result */
        if (selres < 0)
        {
            if (errno != EINTR && errno != EWOULDBLOCK)

diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 5eabcce5da434a593703f291609d257dbe30116a..7dbfa90bf4971f2739448d370a8fdce3f8d766d1 100644 (file)
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -397,9 +397,6 @@
 /* Define to 1 if the assembler supports PPC's LWARX mutex hint bit. */
 #undef HAVE_PPC_LWARX_MUTEX_HINT
 
-/* Define to 1 if you have the `pselect' function. */
-#undef HAVE_PSELECT
-
 /* Define to 1 if you have the `pstat' function. */
 #undef HAVE_PSTAT
 

diff --git a/src/include/pg_config.h.win32 b/src/include/pg_config.h.win32
index 9288e05e07b6b31e66d1824bc685af95fd20b760..216318708284915b83b01c47f0748f0f68ef9d87 100644 (file)
--- a/src/include/pg_config.h.win32
+++ b/src/include/pg_config.h.win32
@@ -264,9 +264,6 @@
 /* Define to 1 if you have the <poll.h> header file. */
 /* #undef HAVE_POLL_H */
 
-/* Define to 1 if you have the `pselect' function. */
-/* #undef HAVE_PSELECT */
-
 /* Define to 1 if you have the `pstat' function. */
 /* #undef HAVE_PSTAT */