Fix race condition between flag checking and signal blocking.

Normally, this case solve to call pselect(2). But Linux's pselect(2)
is emulated by select(2) and sigprocmask(2). It contains this race.
So pgpool archieves using the self-pipe trick.

See 'man pselect' if you want to get more information.

diff --git a/config.h.in b/config.h.in
index aa22c318ece2ec0749cfb6fdc60263d0a86a183d..27a9ba2b4b774bc16c815062f487f84f355120d2 100644 (file)
--- a/config.h.in
+++ b/config.h.in
@@ -147,6 +147,9 @@
 /* Define to 1 if you have the <sys/pstat.h> header file. */
 #undef HAVE_SYS_PSTAT_H
 
+/* Define to 1 if you have the <sys/select.h> header file. */
+#undef HAVE_SYS_SELECT_H
+
 /* Define to 1 if you have the <sys/socket.h> header file. */
 #undef HAVE_SYS_SOCKET_H
 

diff --git a/configure.in b/configure.in
index 7f9c578b654488288958efdb65d4813c2ded33b3..dd4a83d2f6b0abe277566ceaa7644971803ecd6f 100644 (file)
--- a/configure.in
+++ b/configure.in
@@ -24,7 +24,7 @@ AC_CHECK_LIB(resolv,   main)
 dnl Checks for header files.
 AC_HEADER_STDC
 AC_HEADER_SYS_WAIT
-AC_CHECK_HEADERS(fcntl.h unistd.h getopt.h netinet/tcp.h netinet/in.h netdb.h sys/param.h sys/types.h sys/socket.h sys/un.h sys/time.h sys/pstat.h)
+AC_CHECK_HEADERS(fcntl.h unistd.h getopt.h netinet/tcp.h netinet/in.h netdb.h sys/param.h sys/types.h sys/socket.h sys/un.h sys/time.h sys/pstat.h sys/select.h)
 
 dnl Checks for typedefs, structures, and compiler characteristics.
 AC_C_CONST

diff --git a/main.c b/main.c
index 9af9249e4816b1b459b262d8f7d884c04a651698..d87a4a897000689c09775ecd0999c66c195063f0 100644 (file)
--- a/main.c
+++ b/main.c
@@ -21,11 +21,15 @@
 #include "pool.h"
 
 #include <sys/types.h>
+#include <sys/time.h>
 #include <sys/socket.h>
 #include <netinet/in.h>
 #include <sys/un.h>
 #include <netdb.h>
 #include <arpa/inet.h>
+#ifdef HAVE_SYS_SELECT_H
+#include <sys/select.h>
+#endif
 
 #include <sys/stat.h>
 #include <fcntl.h>
@@ -70,6 +74,8 @@ static int create_inet_domain_socket(const char *hostname);
 static void myexit(int code);
 static void failover(int sig);
 static void reaper(void);
+static int pool_pause(struct timeval *timeout);
+static void pool_sleep(unsigned int second);
 
 static RETSIGTYPE exit_handler(int sig);
 static RETSIGTYPE reap_handler(int sig);
@@ -102,8 +108,9 @@ static int stop_sig = SIGTERM;      /* stopping signal default value */
 static int switch_over_sig = SIGUSR1;  /* switch over signal default value */
 
 static int health_check_timer_expired;         /* non 0 if health check timer expired */
-static volatile int failover_request = 0;
-static volatile int sigchld_request = 0;
+static volatile sig_atomic_t failover_request = 0;
+static volatile sig_atomic_t sigchld_request = 0;
+static int pipe_fds[2]; /* for delivering signals */
 
 int myargc;
 char **myargv;
@@ -314,6 +321,7 @@ int main(int argc, char **argv)
        }
 
        /* set up signal handlers */
+       POOL_SETMASK(&BlockSig);
        pool_signal(SIGTERM, exit_handler);
        pool_signal(SIGINT, exit_handler);
        pool_signal(SIGQUIT, exit_handler);
@@ -322,6 +330,13 @@ int main(int argc, char **argv)
        pool_signal(SIGUSR2, failover_handler);
        pool_signal(SIGHUP, exit_handler);
 
+       /* create pipe for delivering event */
+       if (pipe(pipe_fds) < 0)
+       {
+               pool_error("failed to create pipe");
+               myexit(1);
+       }
+
        pool_log("pgpool successfully started");
 
        /*
@@ -376,16 +391,20 @@ int main(int argc, char **argv)
                        }
 
                        sleep_time = pool_config.health_check_period;
-                       while (sleep_time > 0)
-                       {
-                               CHECK_REQUEST;
-                               sleep_time = sleep(sleep_time);
-                       }
+                       pool_sleep(sleep_time);
                }
                else
                {
-                       CHECK_REQUEST;
-                       pause();
+                       for (;;)
+                       {
+                               int r;
+
+                               POOL_SETMASK(&UnBlockSig);
+                               r = pool_pause(NULL);
+                               POOL_SETMASK(&BlockSig);
+                               if (r > 0)
+                                       break;
+                       }
                }
        }
        return 0;
@@ -783,19 +802,21 @@ static RETSIGTYPE exit_handler(int sig)
  */
 static RETSIGTYPE failover_handler(int sig)
 {
-       failover_request = sig;
+       POOL_SETMASK(&BlockSig);
+       failover_request = 1;
+       write(pipe_fds[1], "\0", 1);
+       POOL_SETMASK(&UnBlockSig);
 }
 
 /*
  * Process failover request
+ * failover() must be called under protecting signals.
  */
 static void failover(int sig)
 {
        int i;
        int replication = 0;
 
-       POOL_SETMASK(&BlockSig);
-
        pool_debug("failover_handler called");
 
        /*
@@ -805,7 +826,6 @@ static void failover(int sig)
        if (getpid() != mypid)
        {
                pool_debug("failover_handler: I am not parent");
-               POOL_SETMASK(&UnBlockSig);
                return;
        }
 
@@ -814,7 +834,6 @@ static void failover(int sig)
         */
        if (exiting)
        {
-               POOL_SETMASK(&UnBlockSig);
                return;
        }
 
@@ -823,7 +842,6 @@ static void failover(int sig)
         */
        if (switching)
        {
-               POOL_SETMASK(&UnBlockSig);
                return;
        }
 
@@ -987,7 +1005,6 @@ static void failover(int sig)
 
                switching = 0;
        }
-       POOL_SETMASK(&UnBlockSig);
 }
 
 /*
@@ -1005,30 +1022,32 @@ static RETSIGTYPE health_check_timer_handler(int sig)
  */
 static RETSIGTYPE reap_handler(int sig)
 {
+       POOL_SETMASK(&BlockSig);
        sigchld_request = 1;
+       write(pipe_fds[1], "\0", 1);
+       POOL_SETMASK(&UnBlockSig);
 }
 
-
+/*
+ * Attach zombie processes and restart child processes.
+ * reaper() must be called under protecting signals.
+ */
 static void reaper(void)
 {
        pid_t pid;
        int status;
        int i;
 
-       POOL_SETMASK(&BlockSig);
-
        pool_debug("reap_handler called");
        sigchld_request = 0;
 
        if (exiting)
        {
-               POOL_SETMASK(&UnBlockSig);
                return;
        }
 
        if (switching)
        {
-               POOL_SETMASK(&UnBlockSig);
                return;
        }
 
@@ -1057,6 +1076,59 @@ static void reaper(void)
                                }
                        }
                }
-               POOL_SETMASK(&UnBlockSig);
 
        }
+
+/*
+ * pool_pause: A process pauses by select().
+ */
+static int pool_pause(struct timeval *timeout)
+{
+       fd_set rfds;
+       int n;
+       char dummy;
+
+       FD_ZERO(&rfds);
+       FD_SET(pipe_fds[0], &rfds);
+       n = select(pipe_fds[0]+1, &rfds, NULL, NULL, timeout);
+       if (n == 1)
+               read(pipe_fds[0], &dummy, 1);
+       return n;
+}
+
+/*
+ * pool_pause: A process sleep using pool_pause().
+ *             If a signal event occurs, it raises signal handler.
+ */
+static void pool_sleep(unsigned int second)
+{
+       struct timeval current_time, sleep_time;
+
+       gettimeofday(&current_time, NULL);
+       sleep_time.tv_sec = second + current_time.tv_sec;
+       sleep_time.tv_usec = current_time.tv_usec;
+
+       POOL_SETMASK(&UnBlockSig);
+       while (sleep_time.tv_sec > current_time.tv_sec ||
+                  sleep_time.tv_usec > current_time.tv_usec)
+       {
+               struct timeval timeout;
+               int r;
+
+               timeout.tv_sec = sleep_time.tv_sec - current_time.tv_sec;
+               timeout.tv_usec = sleep_time.tv_usec - current_time.tv_usec;
+               if (timeout.tv_usec < 0)
+               {
+                       timeout.tv_sec--;
+                       timeout.tv_usec += 1000000;
+               }
+
+               r = pool_pause(&timeout);
+               POOL_SETMASK(&BlockSig);
+               if (r > 0)                                                                      
+                       CHECK_REQUEST;
+               POOL_SETMASK(&UnBlockSig);
+               gettimeofday(&current_time, NULL);
+       }
+       POOL_SETMASK(&BlockSig);
+}