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

Commit81069a9

Browse files
committed
Use pselect(2) not select(2), if available, to wait in postmaster's loop.
Traditionally we've unblocked signals, called select(2), and then blockedsignals again. The code expects that the select() will be cancelled withEINTR if an interrupt occurs; but there's a race condition, which is thatan already-pending signal will be delivered as soon as we unblock, and thenwhen we reach select() there will be nothing preventing it from waiting.This can result in a long delay before we perform any action thatServerLoop was supposed to have taken in response to the signal. As withthe somewhat-similar symptoms fixed by commit8939020, the main practicalproblem is slow launching of parallel workers. The window for trouble isusually pretty short, corresponding to one iteration of ServerLoop; butit's not negligible.To fix, use pselect(2) in place of select(2) where available, as that'sdesigned to solve exactly this problem. Where not available, we continueto use the old way, and are no worse off than before.pselect(2) has been required by POSIX since about 2001, so most modernplatforms should have it. A bigger portability issue is that someimplementations are said to be non-atomic, ie pselect() isn't reallyany different from unblock/select/reblock. Still, we're no worse offthan before on such a platform.There is talk of rewriting the postmaster to use a WaitEventSet andnot do signal response work in signal handlers, at which point thiscould be reverted, since we'd be using a self-pipe to solve the racecondition. But that's not happening before v11 at the earliest.Back-patch to 9.6. The problem exists much further back, but theworst symptom arises only in connection with parallel query, so itdoes not seem worth taking any portability risks in older branches.Discussion:https://postgr.es/m/9205.1492833041@sss.pgh.pa.us
1 parent8939020 commit81069a9

File tree

5 files changed

+54
-19
lines changed

5 files changed

+54
-19
lines changed

‎configure

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12892,7 +12892,7 @@ fi
1289212892
LIBS_including_readline="$LIBS"
1289312893
LIBS=`echo"$LIBS"| sed -e's/-ledit//g' -e's/-lreadline//g'`
1289412894

12895-
forac_funcin cbrt clock_gettime dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll pstat pthread_is_threaded_np readlink setproctitle setsid shm_open symlink sync_file_range towlower utime utimes wcstombs wcstombs_l
12895+
forac_funcin cbrt clock_gettime dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove pollpselectpstat pthread_is_threaded_np readlink setproctitle setsid shm_open symlink sync_file_range towlower utime utimes wcstombs wcstombs_l
1289612896
do:
1289712897
as_ac_var=`$as_echo"ac_cv_func_$ac_func"|$as_tr_sh`
1289812898
ac_fn_c_check_func"$LINENO""$ac_func""$as_ac_var"

‎configure.in

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1429,7 +1429,7 @@ PGAC_FUNC_WCSTOMBS_L
14291429
LIBS_including_readline="$LIBS"
14301430
LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
14311431

1432-
AC_CHECK_FUNCS([cbrt clock_gettime dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll pstat pthread_is_threaded_np readlink setproctitle setsid shm_open symlink sync_file_range towlower utime utimes wcstombs wcstombs_l])
1432+
AC_CHECK_FUNCS([cbrt clock_gettime dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove pollpselectpstat pthread_is_threaded_np readlink setproctitle setsid shm_open symlink sync_file_range towlower utime utimes wcstombs wcstombs_l])
14331433

14341434
AC_REPLACE_FUNCS(fseeko)
14351435
case $host_os in

‎src/backend/postmaster/postmaster.c

Lines changed: 46 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -614,9 +614,9 @@ PostmasterMain(int argc, char *argv[])
614614
* In the postmaster, we want to install non-ignored handlers *without*
615615
* SA_RESTART. This is because they'll be blocked at all times except
616616
* when ServerLoop is waiting for something to happen, and during that
617-
* window, we want signals to exit theselect(2) wait so that ServerLoop
617+
* window, we want signals to exit thepselect(2) wait so that ServerLoop
618618
* can respond if anything interesting happened. On some platforms,
619-
* signals marked SA_RESTART would not cause theselect() wait to end.
619+
* signals marked SA_RESTART would not cause thepselect() wait to end.
620620
* Child processes will generally want SA_RESTART, but we expect them to
621621
* set up their own handlers before unblocking signals.
622622
*
@@ -1670,6 +1670,8 @@ ServerLoop(void)
16701670
for (;;)
16711671
{
16721672
fd_setrmask;
1673+
fd_set*rmask_p;
1674+
structtimevaltimeout;
16731675
intselres;
16741676
time_tnow;
16751677

@@ -1679,37 +1681,64 @@ ServerLoop(void)
16791681
* We block all signals except while sleeping. That makes it safe for
16801682
* signal handlers, which again block all signals while executing, to
16811683
* do nontrivial work.
1682-
*
1683-
* If we are in PM_WAIT_DEAD_END state, then we don't want to accept
1684-
* any new connections, so we don't call select(), and just sleep.
16851684
*/
1686-
memcpy((char*)&rmask, (char*)&readmask,sizeof(fd_set));
1687-
16881685
if (pmState==PM_WAIT_DEAD_END)
16891686
{
1690-
PG_SETMASK(&UnBlockSig);
1691-
1692-
pg_usleep(100000L);/* 100 msec seems reasonable */
1693-
selres=0;
1694-
1695-
PG_SETMASK(&BlockSig);
1687+
/*
1688+
* If we are in PM_WAIT_DEAD_END state, then we don't want to
1689+
* accept any new connections, so pass a null rmask.
1690+
*/
1691+
rmask_p=NULL;
1692+
timeout.tv_sec=0;
1693+
timeout.tv_usec=100000;/* 100 msec seems reasonable */
16961694
}
16971695
else
16981696
{
1699-
/* must set timeout each time; some OSes change it! */
1700-
structtimevaltimeout;
1697+
/* Normal case: check sockets, and compute a suitable timeout */
1698+
memcpy(&rmask,&readmask,sizeof(fd_set));
1699+
rmask_p=&rmask;
17011700

17021701
/* Needs to run with blocked signals! */
17031702
DetermineSleepTime(&timeout);
1703+
}
17041704

1705+
/*
1706+
* We prefer to wait with pselect(2) if available, as using that,
1707+
* together with *not* using SA_RESTART for signals, guarantees that
1708+
* we will get kicked off the wait if a signal occurs.
1709+
*
1710+
* If we lack pselect(2), fake it with select(2). This has a race
1711+
* condition: a signal that was already pending will be delivered
1712+
* before we reach the select(), and therefore the select() will wait,
1713+
* even though we might wish to do something in response. Therefore,
1714+
* beware of putting any time-critical signal response logic into
1715+
* ServerLoop rather than into the signal handler itself. It will run
1716+
* eventually, but maybe not till after a timeout delay.
1717+
*
1718+
* Some implementations of pselect() are reportedly not atomic, making
1719+
* the first alternative here functionally equivalent to the second.
1720+
* Not much we can do about that though.
1721+
*/
1722+
{
1723+
#ifdefHAVE_PSELECT
1724+
/* pselect uses a randomly different timeout API, sigh */
1725+
structtimespecptimeout;
1726+
1727+
ptimeout.tv_sec=timeout.tv_sec;
1728+
ptimeout.tv_nsec=timeout.tv_usec*1000;
1729+
1730+
selres=pselect(nSockets,rmask_p,NULL,NULL,
1731+
&ptimeout,&UnBlockSig);
1732+
#else
17051733
PG_SETMASK(&UnBlockSig);
17061734

1707-
selres=select(nSockets,&rmask,NULL,NULL,&timeout);
1735+
selres=select(nSockets,rmask_p,NULL,NULL,&timeout);
17081736

17091737
PG_SETMASK(&BlockSig);
1738+
#endif
17101739
}
17111740

1712-
/* Now check the select() result */
1741+
/* Now check the select()/pselect() result */
17131742
if (selres<0)
17141743
{
17151744
if (errno!=EINTR&&errno!=EWOULDBLOCK)

‎src/include/pg_config.h.in

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,9 @@
400400
/* Define to 1 if the assembler supports PPC's LWARX mutex hint bit. */
401401
#undef HAVE_PPC_LWARX_MUTEX_HINT
402402

403+
/* Define to 1 if you have the `pselect' function. */
404+
#undef HAVE_PSELECT
405+
403406
/* Define to 1 if you have the `pstat' function. */
404407
#undef HAVE_PSTAT
405408

‎src/include/pg_config.h.win32

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,9 @@
267267
/* Define to 1 if you have the <poll.h> header file. */
268268
/* #undef HAVE_POLL_H */
269269

270+
/* Define to 1 if you have the `pselect' function. */
271+
/* #undef HAVE_PSELECT */
272+
270273
/* Define to 1 if you have the `pstat' function. */
271274
/* #undef HAVE_PSTAT */
272275

0 commit comments

Comments
 (0)

[8]ページ先頭

©2009-2025 Movatter.jp