From e42a21b9e6c9b9e6346a34b62628d48ff2fc6ddf Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 14 May 2012 16:11:59 -0400 Subject: [PATCH] Assert that WaitLatchOrSocket callers cannot wait only for writability. Since we have chosen to report socket EOF and error conditions via the WL_SOCKET_READABLE flag bit, it's unsafe to wait only for WL_SOCKET_WRITEABLE; the caller would never be notified of the socket condition, and in some of these implementations WaitLatchOrSocket would busy-wait until something else happens. Add this restriction to the API specification, and add Asserts to check that callers don't try to do that. At some point we might want to consider adjusting the API to relax this restriction, but until we have an actual use case for waiting on a write-only socket, it seems premature to design a solution. --- src/backend/port/unix_latch.c | 10 ++++++++-- src/backend/port/win32_latch.c | 2 ++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/backend/port/unix_latch.c b/src/backend/port/unix_latch.c index 409beaed8d..e64282c210 100644 --- a/src/backend/port/unix_latch.c +++ b/src/backend/port/unix_latch.c @@ -172,6 +172,10 @@ WaitLatch(volatile Latch *latch, int wakeEvents, long timeout) /* * Like WaitLatch, but with an extra socket argument for WL_SOCKET_* * conditions. + * + * When waiting on a socket, WL_SOCKET_READABLE *must* be included in + * 'wakeEvents'; WL_SOCKET_WRITEABLE is optional. The reason for this is + * that EOF and error conditions are reported only via WL_SOCKET_READABLE. */ int WaitLatchOrSocket(volatile Latch *latch, int wakeEvents, pgsocket sock, @@ -195,6 +199,8 @@ WaitLatchOrSocket(volatile Latch *latch, int wakeEvents, pgsocket sock, wakeEvents &= ~(WL_SOCKET_READABLE | WL_SOCKET_WRITEABLE); Assert(wakeEvents != 0); /* must have at least one wake event */ + /* Cannot specify WL_SOCKET_WRITEABLE without WL_SOCKET_READABLE */ + Assert((wakeEvents & (WL_SOCKET_READABLE | WL_SOCKET_WRITEABLE)) != WL_SOCKET_WRITEABLE); if ((wakeEvents & WL_LATCH_SET) && latch->owner_pid != MyProcPid) elog(ERROR, "cannot wait on a latch owned by another process"); @@ -295,7 +301,7 @@ WaitLatchOrSocket(volatile Latch *latch, int wakeEvents, pgsocket sock, if ((wakeEvents & WL_SOCKET_READABLE) && (pfds[0].revents & (POLLIN | POLLHUP | POLLERR | POLLNVAL))) { - /* data available in socket */ + /* data available in socket, or EOF/error condition */ result |= WL_SOCKET_READABLE; } if ((wakeEvents & WL_SOCKET_WRITEABLE) && @@ -373,7 +379,7 @@ WaitLatchOrSocket(volatile Latch *latch, int wakeEvents, pgsocket sock, } if ((wakeEvents & WL_SOCKET_READABLE) && FD_ISSET(sock, &input_mask)) { - /* data available in socket */ + /* data available in socket, or EOF */ result |= WL_SOCKET_READABLE; } if ((wakeEvents & WL_SOCKET_WRITEABLE) && FD_ISSET(sock, &output_mask)) diff --git a/src/backend/port/win32_latch.c b/src/backend/port/win32_latch.c index 62800ad66f..05b34269b5 100644 --- a/src/backend/port/win32_latch.c +++ b/src/backend/port/win32_latch.c @@ -106,6 +106,8 @@ WaitLatchOrSocket(volatile Latch *latch, int wakeEvents, pgsocket sock, wakeEvents &= ~(WL_SOCKET_READABLE | WL_SOCKET_WRITEABLE); Assert(wakeEvents != 0); /* must have at least one wake event */ + /* Cannot specify WL_SOCKET_WRITEABLE without WL_SOCKET_READABLE */ + Assert((wakeEvents & (WL_SOCKET_READABLE | WL_SOCKET_WRITEABLE)) != WL_SOCKET_WRITEABLE); if ((wakeEvents & WL_LATCH_SET) && latch->owner_pid != MyProcPid) elog(ERROR, "cannot wait on a latch owned by another process");