From f40022f1adaeff85b01d98fea38cf5aa16814aa7 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 10 May 2012 14:34:22 -0400 Subject: Make WaitLatch's WL_POSTMASTER_DEATH result trustworthy; simplify callers. Per a suggestion from Peter Geoghegan, make WaitLatch responsible for verifying that the WL_POSTMASTER_DEATH bit it returns is truthful (by testing PostmasterIsAlive). Then simplify its callers, who no longer need to do that for themselves. Remove weasel wording about falsely-set result bits from WaitLatch's API contract. --- src/backend/port/unix_latch.c | 35 ++++++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 11 deletions(-) (limited to 'src/backend/port/unix_latch.c') diff --git a/src/backend/port/unix_latch.c b/src/backend/port/unix_latch.c index 10bf2dbec7e..30d1a488f0a 100644 --- a/src/backend/port/unix_latch.c +++ b/src/backend/port/unix_latch.c @@ -50,6 +50,7 @@ #include "miscadmin.h" #include "postmaster/postmaster.h" #include "storage/latch.h" +#include "storage/pmsignal.h" #include "storage/shmem.h" /* Are we currently in WaitLatch? The signal handler would like to know. */ @@ -160,15 +161,7 @@ DisownLatch(volatile Latch *latch) * * Returns bit mask indicating which condition(s) caused the wake-up. Note * that if multiple wake-up conditions are true, there is no guarantee that - * we return all of them in one call, but we will return at least one. Also, - * according to the select(2) man page on Linux, select(2) may spuriously - * return and report a file descriptor as readable, when it's not. We use - * select(2), so WaitLatch can also spuriously claim that a socket is - * readable, or postmaster has died, even when none of the wake conditions - * have been satisfied. That should be rare in practice, but the caller - * should not use the return value for anything critical, re-checking the - * situation with PostmasterIsAlive() or read() on a socket as necessary. - * The latch and timeout flag bits can be trusted, however. + * we return all of them in one call, but we will return at least one. */ int WaitLatch(volatile Latch *latch, int wakeEvents, long timeout) @@ -318,7 +311,17 @@ WaitLatchOrSocket(volatile Latch *latch, int wakeEvents, pgsocket sock, if ((wakeEvents & WL_POSTMASTER_DEATH) && (pfds[nfds - 1].revents & (POLLHUP | POLLIN | POLLERR | POLLNVAL))) { - result |= WL_POSTMASTER_DEATH; + /* + * According to the select(2) man page on Linux, select(2) may + * spuriously return and report a file descriptor as readable, + * when it's not; and presumably so can poll(2). It's not clear + * that the relevant cases would ever apply to the postmaster + * pipe, but since the consequences of falsely returning + * WL_POSTMASTER_DEATH could be pretty unpleasant, we take the + * trouble to positively verify EOF with PostmasterIsAlive(). + */ + if (!PostmasterIsAlive()) + result |= WL_POSTMASTER_DEATH; } #else /* !HAVE_POLL */ @@ -380,7 +383,17 @@ WaitLatchOrSocket(volatile Latch *latch, int wakeEvents, pgsocket sock, if ((wakeEvents & WL_POSTMASTER_DEATH) && FD_ISSET(postmaster_alive_fds[POSTMASTER_FD_WATCH], &input_mask)) { - result |= WL_POSTMASTER_DEATH; + /* + * According to the select(2) man page on Linux, select(2) may + * spuriously return and report a file descriptor as readable, + * when it's not; and presumably so can poll(2). It's not clear + * that the relevant cases would ever apply to the postmaster + * pipe, but since the consequences of falsely returning + * WL_POSTMASTER_DEATH could be pretty unpleasant, we take the + * trouble to positively verify EOF with PostmasterIsAlive(). + */ + if (!PostmasterIsAlive()) + result |= WL_POSTMASTER_DEATH; } #endif /* HAVE_POLL */ } while (result == 0); -- cgit v1.2.3