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

Commit096ea54

Browse files
committed
Fix race condition in our Windows signal emulation.
pg_signal_dispatch_thread() responded to the client (signal sender)and disconnected the pipe before actually setting the shared variablesthat make the signal visible to the backend process's main thread.In the worst case, it seems, effective delivery of the signal could bepostponed for as long as the machine has any other work to do.To fix, just move the pg_queue_signal() call so that we do it beforeresponding to the client. This essentially makes pgkill() synchronous,which is a stronger guarantee than we have on Unix. That may beoverkill, but on the other hand we have not seen comparable timing bugson any Unix platform.While at it, add some comments to this sadly underdocumented code.Problem diagnosis and fix by Amit Kapila; I just added the comments.Back-patch to all supported versions, as it appears that this can causevisible NOTIFY timing oddities on all of them, and there might beother misbehavior due to slow delivery of other signals.Discussion:https://postgr.es/m/32745.1575303812@sss.pgh.pa.us
1 parentbd0c44b commit096ea54

File tree

1 file changed

+41
-4
lines changed

1 file changed

+41
-4
lines changed

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

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ static pqsigfunc pg_signal_array[PG_SIGNAL_COUNT];
3838
staticpqsigfuncpg_signal_defaults[PG_SIGNAL_COUNT];
3939

4040

41-
/* Signal handling threadfunction */
41+
/* Signal handling threadfunctions */
4242
staticDWORDWINAPIpg_signal_thread(LPVOIDparam);
4343
staticBOOLWINAPIpg_console_handler(DWORDdwCtrlType);
4444

@@ -206,11 +206,14 @@ pgwin32_create_signal_listener(pid_t pid)
206206
*/
207207

208208

209+
/*
210+
* Queue a signal for the main thread, by setting the flag bit and event.
211+
*/
209212
void
210213
pg_queue_signal(intsignum)
211214
{
212215
if (signum >=PG_SIGNAL_COUNT||signum <=0)
213-
return;
216+
return;/* ignore any bad signal number */
214217

215218
EnterCriticalSection(&pg_signal_crit_sec);
216219
pg_signal_queue |=sigmask(signum);
@@ -219,7 +222,11 @@ pg_queue_signal(int signum)
219222
SetEvent(pgwin32_signal_event);
220223
}
221224

222-
/* Signal dispatching thread */
225+
/*
226+
* Signal dispatching thread. This runs after we have received a named
227+
* pipe connection from a client (signal sender). Process the request,
228+
* close the pipe, and exit.
229+
*/
223230
staticDWORDWINAPI
224231
pg_signal_dispatch_thread(LPVOIDparam)
225232
{
@@ -239,13 +246,37 @@ pg_signal_dispatch_thread(LPVOID param)
239246
CloseHandle(pipe);
240247
return0;
241248
}
249+
250+
/*
251+
* Queue the signal before responding to the client. In this way, it's
252+
* guaranteed that once kill() has returned in the signal sender, the next
253+
* CHECK_FOR_INTERRUPTS() in the signal recipient will see the signal.
254+
* (This is a stronger guarantee than POSIX makes; maybe we don't need it?
255+
* But without it, we've seen timing bugs on Windows that do not manifest
256+
* on any known Unix.)
257+
*/
258+
pg_queue_signal(sigNum);
259+
260+
/*
261+
* Write something back to the client, allowing its CallNamedPipe() call
262+
* to terminate.
263+
*/
242264
WriteFile(pipe,&sigNum,1,&bytes,NULL);/* Don't care if it works or
243265
* not.. */
266+
267+
/*
268+
* We must wait for the client to read the data before we can close the
269+
* pipe, else the data will be lost. (If the WriteFile call failed,
270+
* there'll be nothing in the buffer, so this shouldn't block.)
271+
*/
244272
FlushFileBuffers(pipe);
273+
274+
/* This is a formality, since we're about to close the pipe anyway. */
245275
DisconnectNamedPipe(pipe);
276+
277+
/* And we're done. */
246278
CloseHandle(pipe);
247279

248-
pg_queue_signal(sigNum);
249280
return0;
250281
}
251282

@@ -263,6 +294,7 @@ pg_signal_thread(LPVOID param)
263294
BOOLfConnected;
264295
HANDLEhThread;
265296

297+
/* Create a new pipe instance if we don't have one. */
266298
if (pipe==INVALID_HANDLE_VALUE)
267299
{
268300
pipe=CreateNamedPipe(pipename,PIPE_ACCESS_DUPLEX,
@@ -277,6 +309,11 @@ pg_signal_thread(LPVOID param)
277309
}
278310
}
279311

312+
/*
313+
* Wait for a client to connect. If something connects before we
314+
* reach here, we'll get back a "failure" with ERROR_PIPE_CONNECTED,
315+
* which is actually a success (way to go, Microsoft).
316+
*/
280317
fConnected=ConnectNamedPipe(pipe,NULL) ? TRUE : (GetLastError()==ERROR_PIPE_CONNECTED);
281318
if (fConnected)
282319
{

0 commit comments

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