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.
This commit is contained in:
Magnus Hagander 2010-01-31 17:16:25 +00:00
parent 0c969e1fec
commit 27554860c0

View File

@ -6,7 +6,7 @@
* Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group * Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group
* *
* IDENTIFICATION * 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); fConnected = ConnectNamedPipe(pipe, NULL) ? TRUE : (GetLastError() == ERROR_PIPE_CONNECTED);
if (fConnected) 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, hThread = CreateThread(NULL, 0,
(LPTHREAD_START_ROUTINE) pg_signal_dispatch_thread, (LPTHREAD_START_ROUTINE) pg_signal_dispatch_thread,
(LPVOID) pipe, 0, NULL); (LPVOID) pipe, 0, NULL);
@ -283,13 +310,24 @@ pg_signal_thread(LPVOID param)
(int) GetLastError()); (int) GetLastError());
else else
CloseHandle(hThread); 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 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); CloseHandle(pipe);
pipe = INVALID_HANDLE_VALUE;
/* Set up so we create a new pipe on next loop */ }
pipe = INVALID_HANDLE_VALUE;
} }
return 0; return 0;
} }