Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 04a4413

Browse files
committed
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.
1 parent eb88926 commit 04a4413

File tree

1 file changed

+43
-5
lines changed

1 file changed

+43
-5
lines changed

src/backend/port/win32/signal.c

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
77
*
88
* IDENTIFICATION
9-
* $PostgreSQL: pgsql/src/backend/port/win32/signal.c,v 1.23 2010/01/02 16:57:50 momjian Exp $
9+
* $PostgreSQL: pgsql/src/backend/port/win32/signal.c,v 1.24 2010/01/31 17:16:23 mha Exp $
1010
*
1111
*-------------------------------------------------------------------------
1212
*/
@@ -275,6 +275,33 @@ pg_signal_thread(LPVOID param)
275275
fConnected = ConnectNamedPipe(pipe, NULL) ? TRUE : (GetLastError() == ERROR_PIPE_CONNECTED);
276276
if (fConnected)
277277
{
278+
HANDLE newpipe;
279+
280+
/*
281+
* We have a connected pipe. Pass this off to a separate thread that will do the actual
282+
* processing of the pipe.
283+
*
284+
* We must also create a new instance of the pipe *before* we start running the new
285+
* thread. If we don't, there is a race condition whereby the dispatch thread might
286+
* run CloseHandle() before we have created a new instance, thereby causing a small
287+
* window of time where we will miss incoming requests.
288+
*/
289+
newpipe = CreateNamedPipe(pipename, PIPE_ACCESS_DUPLEX,
290+
PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE | PIPE_WAIT,
291+
PIPE_UNLIMITED_INSTANCES, 16, 16, 1000, NULL);
292+
if (newpipe == INVALID_HANDLE_VALUE)
293+
{
294+
/*
295+
* This really should never fail. Just retry in case it does, even though we have
296+
* a small race window in that case. There is nothing else we can do other than
297+
* abort the whole process which will be even worse.
298+
*/
299+
write_stderr("could not create signal listener pipe: error code %d; retrying\n", (int) GetLastError());
300+
/*
301+
* Keep going so we at least dispatch this signal. Hopefully, the call will succeed
302+
* when retried in the loop soon after.
303+
*/
304+
}
278305
hThread = CreateThread(NULL, 0,
279306
(LPTHREAD_START_ROUTINE) pg_signal_dispatch_thread,
280307
(LPVOID) pipe, 0, NULL);
@@ -283,13 +310,24 @@ pg_signal_thread(LPVOID param)
283310
(int) GetLastError());
284311
else
285312
CloseHandle(hThread);
313+
314+
/*
315+
* Background thread is running with our instance of the pipe. So replace our reference
316+
* with the newly created one and loop back up for another run.
317+
*/
318+
pipe = newpipe;
286319
}
287320
else
288-
/* Connection failed. Cleanup and try again */
321+
{
322+
/*
323+
* Connection failed. Cleanup and try again.
324+
*
325+
* This should never happen. If it does, we have a small race condition until we loop
326+
* up and re-create the pipe.
327+
*/
289328
CloseHandle(pipe);
290-
291-
/* Set up so we create a new pipe on next loop */
292-
pipe = INVALID_HANDLE_VALUE;
329+
pipe = INVALID_HANDLE_VALUE;
330+
}
293331
}
294332
return 0;
295333
}

0 commit comments

Comments
 (0)