Fix race condition in win32 signal handling.

There was a race condition where the receiving pipe could be closed by the
child thread if the main thread was pre-empted before it got a chance to
create a new one, and the dispatch thread ran to completion during that time.

One symptom of this is that rows in pg_listener could be dropped under
heavy load.

Analysis and original patch by Radu Ilie, with some small
modifications by Magnus Hagander.

diff --git a/src/backend/port/win32/signal.c b/src/backend/port/win32/signal.c
index d12e29990fd5576581151ca2e2ebc12c0751e0e2..f7195fcf1862df90b59d4d4a02d6f50b5b173f18 100644 (file)
--- a/src/backend/port/win32/signal.c
+++ b/src/backend/port/win32/signal.c
@@ -6,7 +6,7 @@
  * Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/port/win32/signal.c,v 1.17 2006/07/16 20:17:04 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/port/win32/signal.c,v 1.17.2.1 2010/01/31 17:16:25 mha Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -275,6 +275,33 @@ pg_signal_thread(LPVOID param)
        fConnected = ConnectNamedPipe(pipe, NULL) ? TRUE : (GetLastError() == ERROR_PIPE_CONNECTED);
        if (fConnected)
        {
+           HANDLE newpipe;
+
+           /*
+            * We have a connected pipe. Pass this off to a separate thread that will do the actual
+            * processing of the pipe.
+            *
+            * We must also create a new instance of the pipe *before* we start running the new
+            * thread. If we don't, there is a race condition whereby the dispatch thread might
+            * run CloseHandle() before we have created a new instance, thereby causing a small
+            * window of time where we will miss incoming requests.
+            */
+           newpipe = CreateNamedPipe(pipename, PIPE_ACCESS_DUPLEX,
+                                      PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE | PIPE_WAIT,
+                                      PIPE_UNLIMITED_INSTANCES, 16, 16, 1000, NULL);
+           if (newpipe == INVALID_HANDLE_VALUE)
+           {
+               /*
+                * This really should never fail. Just retry in case it does, even though we have
+                * a small race window in that case. There is nothing else we can do other than
+                * abort the whole process which will be even worse.
+                */
+               write_stderr("could not create signal listener pipe: error code %d; retrying\n", (int) GetLastError());
+               /*
+                * Keep going so we at least dispatch this signal. Hopefully, the call will succeed
+                * when retried in the loop soon after.
+                */
+           }
            hThread = CreateThread(NULL, 0,
                          (LPTHREAD_START_ROUTINE) pg_signal_dispatch_thread,
                                   (LPVOID) pipe, 0, NULL);
@@ -283,13 +310,24 @@ pg_signal_thread(LPVOID param)
                             (int) GetLastError());
            else
                CloseHandle(hThread);
+
+           /*
+            * Background thread is running with our instance of the pipe. So replace our reference
+            * with the newly created one and loop back up for another run.
+            */
+           pipe = newpipe;
        }
        else
-           /* Connection failed. Cleanup and try again */
+       {
+           /*
+            * Connection failed. Cleanup and try again.
+            *
+            * This should never happen. If it does, we have a small race condition until we loop
+            * up and re-create the pipe.
+            */
            CloseHandle(pipe);
-
-       /* Set up so we create a new pipe on next loop */
-       pipe = INVALID_HANDLE_VALUE;
+           pipe = INVALID_HANDLE_VALUE;
+       }
    }
    return 0;
 }