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

Commit3e7fdcf

Browse files
committed
Fix WaitLatch() to return promptly when the requested timeout expires.
If the sleep is interrupted by a signal, we must recompute the remainingtime to wait; otherwise, a steady stream of non-wait-terminating interruptscould delay return from WaitLatch indefinitely. This has been shown to bea problem for the autovacuum launcher, and there may well be other placesnow or in the future with similar issues. So we'd better make the functionrobust, even though this'll add at least one gettimeofday call per wait.Back-patch to 9.2. We might eventually need to fix 9.1 as well, but thecode is quite different there, and the usage of WaitLatch in 9.1 is solimited that it's not clearly important to do so.Reported and diagnosed by Jeff Janes, though I rewrote his patch ratherheavily.
1 parentdcc55dd commit3e7fdcf

File tree

3 files changed

+156
-90
lines changed

3 files changed

+156
-90
lines changed

‎src/backend/port/unix_latch.c

Lines changed: 118 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
#endif
4949

5050
#include"miscadmin.h"
51+
#include"portability/instr_time.h"
5152
#include"postmaster/postmaster.h"
5253
#include"storage/latch.h"
5354
#include"storage/pmsignal.h"
@@ -176,12 +177,8 @@ DisownLatch(volatile Latch *latch)
176177
* function returns immediately.
177178
*
178179
* The 'timeout' is given in milliseconds. It must be >= 0 if WL_TIMEOUT flag
179-
* is given. On some platforms, signals do not interrupt the wait, or even
180-
* cause the timeout to be restarted, so beware that the function can sleep
181-
* for several times longer than the requested timeout. However, this
182-
* difficulty is not so great as it seems, because the signal handlers for any
183-
* signals that the caller should respond to ought to be programmed to end the
184-
* wait by calling SetLatch. Ideally, the timeout parameter is vestigial.
180+
* is given. Note that some extra overhead is incurred when WL_TIMEOUT is
181+
* given, so avoid using a timeout if possible.
185182
*
186183
* The latch must be owned by the current process, ie. it must be a
187184
* backend-local latch initialized with InitLatch, or a shared latch
@@ -211,13 +208,16 @@ WaitLatchOrSocket(volatile Latch *latch, int wakeEvents, pgsocket sock,
211208
{
212209
intresult=0;
213210
intrc;
211+
instr_timestart_time,
212+
cur_time;
213+
longcur_timeout;
214214

215215
#ifdefHAVE_POLL
216216
structpollfdpfds[3];
217217
intnfds;
218218
#else
219219
structtimevaltv,
220-
*tvp=NULL;
220+
*tvp;
221221
fd_setinput_mask;
222222
fd_setoutput_mask;
223223
inthifd;
@@ -234,21 +234,30 @@ WaitLatchOrSocket(volatile Latch *latch, int wakeEvents, pgsocket sock,
234234
if ((wakeEvents&WL_LATCH_SET)&&latch->owner_pid!=MyProcPid)
235235
elog(ERROR,"cannot wait on a latch owned by another process");
236236

237-
/* Initialize timeout */
237+
/*
238+
* Initialize timeout if requested. We must record the current time so
239+
* that we can determine the remaining timeout if the poll() or select()
240+
* is interrupted.(On some platforms, select() will update the contents
241+
* of "tv" for us, but unfortunately we can't rely on that.)
242+
*/
238243
if (wakeEvents&WL_TIMEOUT)
239244
{
245+
INSTR_TIME_SET_CURRENT(start_time);
240246
Assert(timeout >=0);
247+
cur_timeout=timeout;
248+
241249
#ifndefHAVE_POLL
242-
tv.tv_sec=timeout /1000L;
243-
tv.tv_usec= (timeout %1000L)*1000L;
250+
tv.tv_sec=cur_timeout /1000L;
251+
tv.tv_usec= (cur_timeout %1000L)*1000L;
244252
tvp=&tv;
245253
#endif
246254
}
247255
else
248256
{
249-
#ifdefHAVE_POLL
250-
/* make sure poll() agrees there is no timeout */
251-
timeout=-1;
257+
cur_timeout=-1;
258+
259+
#ifndefHAVE_POLL
260+
tvp=NULL;
252261
#endif
253262
}
254263

@@ -311,54 +320,62 @@ WaitLatchOrSocket(volatile Latch *latch, int wakeEvents, pgsocket sock,
311320
}
312321

313322
/* Sleep */
314-
rc=poll(pfds,nfds, (int)timeout);
323+
rc=poll(pfds,nfds, (int)cur_timeout);
315324

316325
/* Check return code */
317326
if (rc<0)
318327
{
319-
if (errno==EINTR)
320-
continue;
321-
waiting= false;
322-
ereport(ERROR,
323-
(errcode_for_socket_access(),
324-
errmsg("poll() failed: %m")));
328+
/* EINTR is okay, otherwise complain */
329+
if (errno!=EINTR)
330+
{
331+
waiting= false;
332+
ereport(ERROR,
333+
(errcode_for_socket_access(),
334+
errmsg("poll() failed: %m")));
335+
}
325336
}
326-
if (rc==0&& (wakeEvents&WL_TIMEOUT))
337+
elseif (rc==0)
327338
{
328339
/* timeout exceeded */
329-
result |=WL_TIMEOUT;
340+
if (wakeEvents&WL_TIMEOUT)
341+
result |=WL_TIMEOUT;
330342
}
331-
if ((wakeEvents&WL_SOCKET_READABLE)&&
332-
(pfds[0].revents& (POLLIN |POLLHUP |POLLERR |POLLNVAL)))
343+
else
333344
{
334-
/* data available in socket, or EOF/error condition */
335-
result |=WL_SOCKET_READABLE;
336-
}
337-
if ((wakeEvents&WL_SOCKET_WRITEABLE)&&
338-
(pfds[0].revents&POLLOUT))
339-
{
340-
result |=WL_SOCKET_WRITEABLE;
341-
}
345+
/* at least one event occurred, so check revents values */
346+
if ((wakeEvents&WL_SOCKET_READABLE)&&
347+
(pfds[0].revents& (POLLIN |POLLHUP |POLLERR |POLLNVAL)))
348+
{
349+
/* data available in socket, or EOF/error condition */
350+
result |=WL_SOCKET_READABLE;
351+
}
352+
if ((wakeEvents&WL_SOCKET_WRITEABLE)&&
353+
(pfds[0].revents&POLLOUT))
354+
{
355+
result |=WL_SOCKET_WRITEABLE;
356+
}
342357

343-
/*
344-
* We expect a POLLHUP when the remote end is closed, but because we
345-
* don't expect the pipe to become readable or to have any errors
346-
* either, treat those as postmaster death, too.
347-
*/
348-
if ((wakeEvents&WL_POSTMASTER_DEATH)&&
349-
(pfds[nfds-1].revents& (POLLHUP |POLLIN |POLLERR |POLLNVAL)))
350-
{
351358
/*
352-
* According to the select(2) man page on Linux, select(2) may
353-
* spuriously return and report a file descriptor as readable,
354-
* when it's not; and presumably so can poll(2). It's not clear
355-
* that the relevant cases would ever apply to the postmaster
356-
* pipe, but since the consequences of falsely returning
357-
* WL_POSTMASTER_DEATH could be pretty unpleasant, we take the
358-
* trouble to positively verify EOF with PostmasterIsAlive().
359+
* We expect a POLLHUP when the remote end is closed, but because
360+
* we don't expect the pipe to become readable or to have any
361+
* errors either, treat those cases as postmaster death, too.
359362
*/
360-
if (!PostmasterIsAlive())
361-
result |=WL_POSTMASTER_DEATH;
363+
if ((wakeEvents&WL_POSTMASTER_DEATH)&&
364+
(pfds[nfds-1].revents& (POLLHUP |POLLIN |POLLERR |POLLNVAL)))
365+
{
366+
/*
367+
* According to the select(2) man page on Linux, select(2) may
368+
* spuriously return and report a file descriptor as readable,
369+
* when it's not; and presumably so can poll(2). It's not
370+
* clear that the relevant cases would ever apply to the
371+
* postmaster pipe, but since the consequences of falsely
372+
* returning WL_POSTMASTER_DEATH could be pretty unpleasant,
373+
* we take the trouble to positively verify EOF with
374+
* PostmasterIsAlive().
375+
*/
376+
if (!PostmasterIsAlive())
377+
result |=WL_POSTMASTER_DEATH;
378+
}
362379
}
363380
#else/* !HAVE_POLL */
364381

@@ -395,43 +412,66 @@ WaitLatchOrSocket(volatile Latch *latch, int wakeEvents, pgsocket sock,
395412
/* Check return code */
396413
if (rc<0)
397414
{
398-
if (errno==EINTR)
399-
continue;
400-
waiting= false;
401-
ereport(ERROR,
402-
(errcode_for_socket_access(),
403-
errmsg("select() failed: %m")));
415+
/* EINTR is okay, otherwise complain */
416+
if (errno!=EINTR)
417+
{
418+
waiting= false;
419+
ereport(ERROR,
420+
(errcode_for_socket_access(),
421+
errmsg("select() failed: %m")));
422+
}
404423
}
405-
if (rc==0&& (wakeEvents&WL_TIMEOUT))
424+
elseif (rc==0)
406425
{
407426
/* timeout exceeded */
408-
result |=WL_TIMEOUT;
409-
}
410-
if ((wakeEvents&WL_SOCKET_READABLE)&&FD_ISSET(sock,&input_mask))
411-
{
412-
/* data available in socket, or EOF */
413-
result |=WL_SOCKET_READABLE;
427+
if (wakeEvents&WL_TIMEOUT)
428+
result |=WL_TIMEOUT;
414429
}
415-
if ((wakeEvents&WL_SOCKET_WRITEABLE)&&FD_ISSET(sock,&output_mask))
430+
else
416431
{
417-
result |=WL_SOCKET_WRITEABLE;
418-
}
419-
if ((wakeEvents&WL_POSTMASTER_DEATH)&&
432+
/* at least one event occurred, so check masks */
433+
if ((wakeEvents&WL_SOCKET_READABLE)&&FD_ISSET(sock,&input_mask))
434+
{
435+
/* data available in socket, or EOF */
436+
result |=WL_SOCKET_READABLE;
437+
}
438+
if ((wakeEvents&WL_SOCKET_WRITEABLE)&&FD_ISSET(sock,&output_mask))
439+
{
440+
result |=WL_SOCKET_WRITEABLE;
441+
}
442+
if ((wakeEvents&WL_POSTMASTER_DEATH)&&
420443
FD_ISSET(postmaster_alive_fds[POSTMASTER_FD_WATCH],&input_mask))
421-
{
422-
/*
423-
* According to the select(2) man page on Linux, select(2) may
424-
* spuriously return and report a file descriptor as readable,
425-
* when it's not; and presumably so can poll(2). It's not clear
426-
* that the relevant cases would ever apply to the postmaster
427-
* pipe, but since the consequences of falsely returning
428-
* WL_POSTMASTER_DEATH could be pretty unpleasant, we take the
429-
* trouble to positively verify EOF with PostmasterIsAlive().
430-
*/
431-
if (!PostmasterIsAlive())
432-
result |=WL_POSTMASTER_DEATH;
444+
{
445+
/*
446+
* According to the select(2) man page on Linux, select(2) may
447+
* spuriously return and report a file descriptor as readable,
448+
* when it's not; and presumably so can poll(2). It's not
449+
* clear that the relevant cases would ever apply to the
450+
* postmaster pipe, but since the consequences of falsely
451+
* returning WL_POSTMASTER_DEATH could be pretty unpleasant,
452+
* we take the trouble to positively verify EOF with
453+
* PostmasterIsAlive().
454+
*/
455+
if (!PostmasterIsAlive())
456+
result |=WL_POSTMASTER_DEATH;
457+
}
433458
}
434459
#endif/* HAVE_POLL */
460+
461+
/* If we're not done, update cur_timeout for next iteration */
462+
if (result==0&&cur_timeout >=0)
463+
{
464+
INSTR_TIME_SET_CURRENT(cur_time);
465+
INSTR_TIME_SUBTRACT(cur_time,start_time);
466+
cur_timeout=timeout- (long)INSTR_TIME_GET_MILLISEC(cur_time);
467+
if (cur_timeout<0)
468+
cur_timeout=0;
469+
470+
#ifndefHAVE_POLL
471+
tv.tv_sec=cur_timeout /1000L;
472+
tv.tv_usec= (cur_timeout %1000L)*1000L;
473+
#endif
474+
}
435475
}while (result==0);
436476
waiting= false;
437477

‎src/backend/port/win32_latch.c

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include<unistd.h>
2525

2626
#include"miscadmin.h"
27+
#include"portability/instr_time.h"
2728
#include"postmaster/postmaster.h"
2829
#include"storage/latch.h"
2930
#include"storage/pmsignal.h"
@@ -100,6 +101,9 @@ WaitLatchOrSocket(volatile Latch *latch, int wakeEvents, pgsocket sock,
100101
longtimeout)
101102
{
102103
DWORDrc;
104+
instr_timestart_time,
105+
cur_time;
106+
longcur_timeout;
103107
HANDLEevents[4];
104108
HANDLElatchevent;
105109
HANDLEsockevent=WSA_INVALID_EVENT;
@@ -118,11 +122,19 @@ WaitLatchOrSocket(volatile Latch *latch, int wakeEvents, pgsocket sock,
118122
if ((wakeEvents&WL_LATCH_SET)&&latch->owner_pid!=MyProcPid)
119123
elog(ERROR,"cannot wait on a latch owned by another process");
120124

121-
/* Convert timeout to form used by WaitForMultipleObjects() */
125+
/*
126+
* Initialize timeout if requested. We must record the current time so
127+
* that we can determine the remaining timeout if WaitForMultipleObjects
128+
* is interrupted.
129+
*/
122130
if (wakeEvents&WL_TIMEOUT)
131+
{
132+
INSTR_TIME_SET_CURRENT(start_time);
123133
Assert(timeout >=0);
134+
cur_timeout=timeout;
135+
}
124136
else
125-
timeout=INFINITE;
137+
cur_timeout=INFINITE;
126138

127139
/*
128140
* Construct an array of event handles for WaitforMultipleObjects().
@@ -187,7 +199,7 @@ WaitLatchOrSocket(volatile Latch *latch, int wakeEvents, pgsocket sock,
187199
break;
188200
}
189201

190-
rc=WaitForMultipleObjects(numevents,events, FALSE,timeout);
202+
rc=WaitForMultipleObjects(numevents,events, FALSE,cur_timeout);
191203

192204
if (rc==WAIT_FAILED)
193205
elog(ERROR,"WaitForMultipleObjects() failed: error code %lu",
@@ -203,7 +215,11 @@ WaitLatchOrSocket(volatile Latch *latch, int wakeEvents, pgsocket sock,
203215
}
204216
elseif (rc==WAIT_OBJECT_0+1)
205217
{
206-
/* Latch is set, we'll handle that on next iteration of loop */
218+
/*
219+
* Latch is set. We'll handle that on next iteration of loop, but
220+
* let's not waste the cycles to update cur_timeout below.
221+
*/
222+
continue;
207223
}
208224
elseif ((wakeEvents& (WL_SOCKET_READABLE |WL_SOCKET_WRITEABLE))&&
209225
rc==WAIT_OBJECT_0+2)/* socket is at event slot 2 */
@@ -240,8 +256,17 @@ WaitLatchOrSocket(volatile Latch *latch, int wakeEvents, pgsocket sock,
240256
}
241257
else
242258
elog(ERROR,"unexpected return code from WaitForMultipleObjects(): %lu",rc);
243-
}
244-
while (result==0);
259+
260+
/* If we're not done, update cur_timeout for next iteration */
261+
if (result==0&&cur_timeout!=INFINITE)
262+
{
263+
INSTR_TIME_SET_CURRENT(cur_time);
264+
INSTR_TIME_SUBTRACT(cur_time,start_time);
265+
cur_timeout=timeout- (long)INSTR_TIME_GET_MILLISEC(cur_time);
266+
if (cur_timeout<0)
267+
cur_timeout=0;
268+
}
269+
}while (result==0);
245270

246271
/* Clean up the event object we created for the socket */
247272
if (sockevent!=WSA_INVALID_EVENT)

‎src/include/storage/latch.h

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@
3333
* ResetLatch- Clears the latch, allowing it to be set again
3434
* WaitLatch- Waits for the latch to become set
3535
*
36-
* WaitLatch includes a provision for timeouts (which shouldhopefully not
37-
*be necessary once the code is fully latch-ified) and a provision for
36+
* WaitLatch includes a provision for timeouts (which shouldbe avoided
37+
*when possible, as they incur extra overhead) and a provision for
3838
* postmaster child processes to wake up immediately on postmaster death.
3939
* See unix_latch.c for detailed specifications for the exported functions.
4040
*
@@ -64,14 +64,15 @@
6464
* will be lifted in future by inserting suitable memory barriers into
6565
* SetLatch and ResetLatch.
6666
*
67+
* On some platforms, signals will not interrupt the latch wait primitive
68+
* by themselves. Therefore, it is critical that any signal handler that
69+
* is meant to terminate a WaitLatch wait calls SetLatch.
70+
*
6771
* Note that use of the process latch (PGPROC.procLatch) is generally better
6872
* than an ad-hoc shared latch for signaling auxiliary processes. This is
6973
* because generic signal handlers will call SetLatch on the process latch
7074
* only, so using any latch other than the process latch effectively precludes
71-
* ever registering a generic handler.Since signals have the potential to
72-
* invalidate the latch timeout on some platforms, resulting in a
73-
* denial-of-service, it is important to verify that all signal handlers
74-
* within all WaitLatch-calling processes call SetLatch.
75+
* use of any generic handler.
7576
*
7677
*
7778
* Portions Copyright (c) 1996-2012, PostgreSQL Global Development Group

0 commit comments

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