Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit16114f2

Browse files
committed
Use only one thread to handle incoming signals on Windows.
Since its inception, our Windows signal emulation code has worked byrunning a main signal thread that just watches for incoming signalrequests, and then spawns a new thread to handle each such request.That design is meant for servers in which requests can take substantialeffort to process, and it's worth parallelizing the handling ofrequests. But those assumptions are just bogus for our signal code.It's not much more than pg_queue_signal(), which is cheap and can'tparallelize at all, plus we don't really expect lots of signals toarrive at the same backend at once. More importantly, this approachcreates failure modes that we could do without: either inability tospawn a new thread or inability to create a new pipe handle will riskloss of signals. Hence, dispense with the separate per-signal threadsand just service each request in-line in the main signal thread. Thisshould be a bit faster (for the normal case of one signal at a time)as well as more robust.Patch by me; thanks to Andrew Dunstan for testing and Amit Kapilafor review.Discussion:https://postgr.es/m/4412.1575748586@sss.pgh.pa.us
1 parent105eb36 commit16114f2

File tree

1 file changed

+41
-96
lines changed

1 file changed

+41
-96
lines changed

‎src/backend/port/win32/signal.c

Lines changed: 41 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -225,77 +225,19 @@ pg_queue_signal(int signum)
225225
SetEvent(pgwin32_signal_event);
226226
}
227227

228-
/*
229-
* Signal dispatching thread. This runs after we have received a named
230-
* pipe connection from a client (signal sender). Process the request,
231-
* close the pipe, and exit.
232-
*/
233-
staticDWORDWINAPI
234-
pg_signal_dispatch_thread(LPVOIDparam)
235-
{
236-
HANDLEpipe= (HANDLE)param;
237-
BYTEsigNum;
238-
DWORDbytes;
239-
240-
if (!ReadFile(pipe,&sigNum,1,&bytes,NULL))
241-
{
242-
/* Client died before sending */
243-
CloseHandle(pipe);
244-
return0;
245-
}
246-
if (bytes!=1)
247-
{
248-
/* Received <bytes> bytes over signal pipe (should be 1) */
249-
CloseHandle(pipe);
250-
return0;
251-
}
252-
253-
/*
254-
* Queue the signal before responding to the client. In this way, it's
255-
* guaranteed that once kill() has returned in the signal sender, the next
256-
* CHECK_FOR_INTERRUPTS() in the signal recipient will see the signal.
257-
* (This is a stronger guarantee than POSIX makes; maybe we don't need it?
258-
* But without it, we've seen timing bugs on Windows that do not manifest
259-
* on any known Unix.)
260-
*/
261-
pg_queue_signal(sigNum);
262-
263-
/*
264-
* Write something back to the client, allowing its CallNamedPipe() call
265-
* to terminate.
266-
*/
267-
WriteFile(pipe,&sigNum,1,&bytes,NULL);/* Don't care if it works or
268-
* not.. */
269-
270-
/*
271-
* We must wait for the client to read the data before we can close the
272-
* pipe, else the data will be lost. (If the WriteFile call failed,
273-
* there'll be nothing in the buffer, so this shouldn't block.)
274-
*/
275-
FlushFileBuffers(pipe);
276-
277-
/* This is a formality, since we're about to close the pipe anyway. */
278-
DisconnectNamedPipe(pipe);
279-
280-
/* And we're done. */
281-
CloseHandle(pipe);
282-
283-
return0;
284-
}
285-
286228
/* Signal handling thread */
287229
staticDWORDWINAPI
288230
pg_signal_thread(LPVOIDparam)
289231
{
290232
charpipename[128];
291233
HANDLEpipe=pgwin32_initial_signal_pipe;
292234

235+
/* Set up pipe name, in case we have to re-create the pipe. */
293236
snprintf(pipename,sizeof(pipename),"\\\\.\\pipe\\pgsignal_%lu",GetCurrentProcessId());
294237

295238
for (;;)
296239
{
297240
BOOLfConnected;
298-
HANDLEhThread;
299241

300242
/* Create a new pipe instance if we don't have one. */
301243
if (pipe==INVALID_HANDLE_VALUE)
@@ -320,59 +262,62 @@ pg_signal_thread(LPVOID param)
320262
fConnected=ConnectNamedPipe(pipe,NULL) ? TRUE : (GetLastError()==ERROR_PIPE_CONNECTED);
321263
if (fConnected)
322264
{
323-
HANDLEnewpipe;
324-
325265
/*
326-
* We have a connected pipe. Pass this off to a separate thread
327-
* that will do the actual processing of the pipe.
328-
*
329-
* We must also create a new instance of the pipe *before* we
330-
* start running the new thread. If we don't, there is a race
331-
* condition whereby the dispatch thread might run CloseHandle()
332-
* before we have created a new instance, thereby causing a small
333-
* window of time where we will miss incoming requests.
266+
* We have a connection from a would-be signal sender. Process it.
334267
*/
335-
newpipe=CreateNamedPipe(pipename,PIPE_ACCESS_DUPLEX,
336-
PIPE_TYPE_MESSAGE |PIPE_READMODE_MESSAGE |PIPE_WAIT,
337-
PIPE_UNLIMITED_INSTANCES,16,16,1000,NULL);
338-
if (newpipe==INVALID_HANDLE_VALUE)
268+
BYTEsigNum;
269+
DWORDbytes;
270+
271+
if (ReadFile(pipe,&sigNum,1,&bytes,NULL)&&
272+
bytes==1)
339273
{
340274
/*
341-
* This really should never fail. Just retry in case it does,
342-
* even though we have a small race window in that case. There
343-
* is nothing else we can do other than abort the whole
344-
* process which will be even worse.
275+
* Queue the signal before responding to the client. In this
276+
* way, it's guaranteed that once kill() has returned in the
277+
* signal sender, the next CHECK_FOR_INTERRUPTS() in the
278+
* signal recipient will see the signal. (This is a stronger
279+
* guarantee than POSIX makes; maybe we don't need it? But
280+
* without it, we've seen timing bugs on Windows that do not
281+
* manifest on any known Unix.)
345282
*/
346-
write_stderr("could not create signal listener pipe: error code %lu; retrying\n",GetLastError());
283+
pg_queue_signal(sigNum);
347284

348285
/*
349-
*Keep going so we at least dispatch this signal. Hopefully,
350-
*the callwill succeed when retried in the loop soon after.
286+
*Write something back to the client, allowing its
287+
*CallNamedPipe() callto terminate.
351288
*/
289+
WriteFile(pipe,&sigNum,1,&bytes,NULL);/* Don't care if it
290+
* works or not */
291+
292+
/*
293+
* We must wait for the client to read the data before we can
294+
* disconnect, else the data will be lost. (If the WriteFile
295+
* call failed, there'll be nothing in the buffer, so this
296+
* shouldn't block.)
297+
*/
298+
FlushFileBuffers(pipe);
352299
}
353-
hThread=CreateThread(NULL,0,
354-
(LPTHREAD_START_ROUTINE)pg_signal_dispatch_thread,
355-
(LPVOID)pipe,0,NULL);
356-
if (hThread==INVALID_HANDLE_VALUE)
357-
write_stderr("could not create signal dispatch thread: error code %lu\n",
358-
GetLastError());
359300
else
360-
CloseHandle(hThread);
301+
{
302+
/*
303+
* If we fail to read a byte from the client, assume it's the
304+
* client's problem and do nothing. Perhaps it'd be better to
305+
* force a pipe close and reopen?
306+
*/
307+
}
361308

362-
/*
363-
* Background thread is running with our instance of the pipe. So
364-
* replace our reference with the newly created one and loop back
365-
* up for another run.
366-
*/
367-
pipe=newpipe;
309+
/* Disconnect from client so that we can re-use the pipe. */
310+
DisconnectNamedPipe(pipe);
368311
}
369312
else
370313
{
371314
/*
372-
* Connection failed. Cleanup and try again.
315+
* Connection failed.Cleanup and try again.
373316
*
374-
* This should never happen. If it does, we have a small race
375-
* condition until we loop up and re-create the pipe.
317+
* This should never happen. If it does, there's a window where
318+
* we'll miss signals until we manage to re-create the pipe.
319+
* However, just trying to use the same pipe again is probably not
320+
* going to work, so we have little choice.
376321
*/
377322
CloseHandle(pipe);
378323
pipe=INVALID_HANDLE_VALUE;

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp