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

Commit28e6a2f

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 parent99351a8 commit28e6a2f

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

@@ -208,12 +208,15 @@ pgwin32_create_signal_listener(pid_t pid)
208208
*/
209209

210210

211+
/*
212+
* Queue a signal for the main thread, by setting the flag bit and event.
213+
*/
211214
void
212215
pg_queue_signal(intsignum)
213216
{
214217
Assert(pgwin32_signal_event!=NULL);
215218
if (signum >=PG_SIGNAL_COUNT||signum <=0)
216-
return;
219+
return;/* ignore any bad signal number */
217220

218221
EnterCriticalSection(&pg_signal_crit_sec);
219222
pg_signal_queue |=sigmask(signum);
@@ -222,7 +225,11 @@ pg_queue_signal(int signum)
222225
SetEvent(pgwin32_signal_event);
223226
}
224227

225-
/* Signal dispatching thread */
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+
*/
226233
staticDWORDWINAPI
227234
pg_signal_dispatch_thread(LPVOIDparam)
228235
{
@@ -242,13 +249,37 @@ pg_signal_dispatch_thread(LPVOID param)
242249
CloseHandle(pipe);
243250
return0;
244251
}
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+
*/
245267
WriteFile(pipe,&sigNum,1,&bytes,NULL);/* Don't care if it works or
246268
* 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+
*/
247275
FlushFileBuffers(pipe);
276+
277+
/* This is a formality, since we're about to close the pipe anyway. */
248278
DisconnectNamedPipe(pipe);
279+
280+
/* And we're done. */
249281
CloseHandle(pipe);
250282

251-
pg_queue_signal(sigNum);
252283
return0;
253284
}
254285

@@ -266,6 +297,7 @@ pg_signal_thread(LPVOID param)
266297
BOOLfConnected;
267298
HANDLEhThread;
268299

300+
/* Create a new pipe instance if we don't have one. */
269301
if (pipe==INVALID_HANDLE_VALUE)
270302
{
271303
pipe=CreateNamedPipe(pipename,PIPE_ACCESS_DUPLEX,
@@ -280,6 +312,11 @@ pg_signal_thread(LPVOID param)
280312
}
281313
}
282314

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

0 commit comments

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