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

Commit9b6e0b9

Browse files
committed
Fix waitpid() emulation on Windows.
Our waitpid() emulation didn't prevent a PID from being recycled by theOS before the call to waitpid(). The postmaster could finish uptracking more than one child process with the same PID, and confusethem.Fix, by moving the guts of pgwin32_deadchild_callback() into waitpid(),so that resources are released synchronously. The process and PIDcontinue to exist until we close the process handle, which only happensonce we're ready to adjust our book-keeping of running children.This seems to explain a couple of failures on CI. It had never beenreported before, despite the code being as old as the Windows port.Perhaps Windows started recycling PIDs more rapidly, or perhaps timingchanges due to commit7389aad made it more likely to break.Thanks to Alexander Lakhin for analysis and Andres Freund for trackingdown the root cause.Back-patch to all supported branches.Reported-by: Andres Freund <andres@anarazel.de>Discussion:https://postgr.es/m/20230208012852.bvkn2am4h4iqjogq%40awork3.anarazel.de
1 parent7cac191 commit9b6e0b9

File tree

1 file changed

+40
-30
lines changed

1 file changed

+40
-30
lines changed

‎src/backend/postmaster/postmaster.c

Lines changed: 40 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -4887,7 +4887,7 @@ internal_forkexec(int argc, char *argv[], Port *port)
48874887
(errmsg_internal("could not register process for wait: error code %lu",
48884888
GetLastError())));
48894889

4890-
/* Don't close pi.hProcess here -the wait thread needs access to it */
4890+
/* Don't close pi.hProcess here -waitpid() needs access to it */
48914891

48924892
CloseHandle(pi.hThread);
48934893

@@ -6531,36 +6531,21 @@ ShmemBackendArrayRemove(Backend *bn)
65316531
staticpid_t
65326532
waitpid(pid_tpid,int*exitstatus,intoptions)
65336533
{
6534+
win32_deadchild_waitinfo*childinfo;
6535+
DWORDexitcode;
65346536
DWORDdwd;
65356537
ULONG_PTRkey;
65366538
OVERLAPPED*ovl;
65376539

6538-
/*
6539-
* Check if there are any dead children. If there are, return the pid of
6540-
* the first one that died.
6541-
*/
6542-
if (GetQueuedCompletionStatus(win32ChildQueue,&dwd,&key,&ovl,0))
6540+
/* Try to consume one win32_deadchild_waitinfo from the queue. */
6541+
if (!GetQueuedCompletionStatus(win32ChildQueue,&dwd,&key,&ovl,0))
65436542
{
6544-
*exitstatus=(int)key;
6545-
returndwd;
6543+
errno=EAGAIN;
6544+
return-1;
65466545
}
65476546

6548-
return-1;
6549-
}
6550-
6551-
/*
6552-
* Note! Code below executes on a thread pool! All operations must
6553-
* be thread safe! Note that elog() and friends must *not* be used.
6554-
*/
6555-
staticvoidWINAPI
6556-
pgwin32_deadchild_callback(PVOIDlpParameter,BOOLEANTimerOrWaitFired)
6557-
{
6558-
win32_deadchild_waitinfo*childinfo= (win32_deadchild_waitinfo*)lpParameter;
6559-
DWORDexitcode;
6560-
6561-
if (TimerOrWaitFired)
6562-
return;/* timeout. Should never happen, since we use
6563-
* INFINITE as timeout value. */
6547+
childinfo= (win32_deadchild_waitinfo*)key;
6548+
pid=childinfo->procId;
65646549

65656550
/*
65666551
* Remove handle from wait - required even though it's set to wait only
@@ -6576,13 +6561,11 @@ pgwin32_deadchild_callback(PVOID lpParameter, BOOLEAN TimerOrWaitFired)
65766561
write_stderr("could not read exit code for process\n");
65776562
exitcode=255;
65786563
}
6579-
6580-
if (!PostQueuedCompletionStatus(win32ChildQueue,childinfo->procId, (ULONG_PTR)exitcode,NULL))
6581-
write_stderr("could not post child completion status\n");
6564+
*exitstatus=exitcode;
65826565

65836566
/*
6584-
*Handle is per-process, so we close it here instead of inthe
6585-
*originating thread
6567+
*Close theprocess handle. Only after this point canthe PID can be
6568+
*recycled by the kernel.
65866569
*/
65876570
CloseHandle(childinfo->procHandle);
65886571

@@ -6592,7 +6575,34 @@ pgwin32_deadchild_callback(PVOID lpParameter, BOOLEAN TimerOrWaitFired)
65926575
*/
65936576
free(childinfo);
65946577

6595-
/* Queue SIGCHLD signal */
6578+
returnpid;
6579+
}
6580+
6581+
/*
6582+
* Note! Code below executes on a thread pool! All operations must
6583+
* be thread safe! Note that elog() and friends must *not* be used.
6584+
*/
6585+
staticvoidWINAPI
6586+
pgwin32_deadchild_callback(PVOIDlpParameter,BOOLEANTimerOrWaitFired)
6587+
{
6588+
/* Should never happen, since we use INFINITE as timeout value. */
6589+
if (TimerOrWaitFired)
6590+
return;
6591+
6592+
/*
6593+
* Post the win32_deadchild_waitinfo object for waitpid() to deal with. If
6594+
* that fails, we leak the object, but we also leak a whole process and
6595+
* get into an unrecoverable state, so there's not much point in worrying
6596+
* about that. We'd like to panic, but we can't use that infrastructure
6597+
* from this thread.
6598+
*/
6599+
if (!PostQueuedCompletionStatus(win32ChildQueue,
6600+
0,
6601+
(ULONG_PTR)lpParameter,
6602+
NULL))
6603+
write_stderr("could not post child completion status\n");
6604+
6605+
/* Queue SIGCHLD signal. */
65966606
pg_queue_signal(SIGCHLD);
65976607
}
65986608
#endif/* WIN32 */

0 commit comments

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