Process 'die' interrupts while reading/writing from the client socket.

Up to now it was impossible to terminate a backend that was trying to
send/recv data to/from the client when the socket's buffer was already
full/empty. While the send/recv calls itself might have gotten
interrupted by signals on some platforms, we just immediately retried.

That could lead to situations where a backend couldn't be terminated ,
after a client died without the connection being closed, because it
was blocked in send/recv.

The problem was far more likely to be hit when sending data than when
reading. That's because while reading a command from the client, and
during authentication, we processed interrupts immediately . That
primarily left COPY FROM STDIN as being problematic for recv.

Change things so that that we process 'die' events immediately when
the appropriate signal arrives. We can't sensibly react to query
cancels at that point, because we might loose sync with the client as
we could be in the middle of writing a message.

We don't interrupt writes if the write buffer isn't full, as indicated
by write() returning EWOULDBLOCK, as that would lead to fewer error
messages reaching clients.

Per discussion with Kyotaro HORIGUCHI and Heikki Linnakangas

Discussion: 20140927191243.GD5423@alap3.anarazel.de

1 files changed, 37 insertions, 21 deletions

diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c
index b90ab0ea86f..c2c1842eb8e 100644
--- a/src/backend/libpq/be-secure.c
+++ b/src/backend/libpq/be-secure.c
@@ -140,14 +140,27 @@ retry:
 		n = secure_raw_read(port, ptr, len);
 	}
 
-	/* Process interrupts that happened while (or before) receiving. */
-	ProcessClientReadInterrupt(); /* preserves errno */
-
 	/* retry after processing interrupts */
 	if (n < 0 && errno == EINTR)
 	{
+		/*
+		 * We tried to read data, the socket was empty, and we were
+		 * interrupted while waiting for readability. We only process
+		 * interrupts if we got interrupted while reading and when in blocking
+		 * mode. In other cases it's better to allow the interrupts to be
+		 * handled at higher layers.
+		 */
+		ProcessClientReadInterrupt(!port->noblock); /* preserves errno */
 		goto retry;
 	}
+
+	/*
+	 * Process interrupts that happened while (or before) receiving. Note that
+	 * we signal that we're not blocking, which will prevent some types of
+	 * interrupts from being processed.
+	 */
+	ProcessClientReadInterrupt(false);
+
 	return n;
 }
 
@@ -224,18 +237,17 @@ retry:
 		n = secure_raw_write(port, ptr, len);
 	}
 
-	/*
-	 * XXX: We'll, at some later point, likely want to add interrupt
-	 * processing here.
-	 */
-
-	/*
-	 * Retry after processing interrupts. This can be triggered even though we
-	 * don't check for latch set's during writing yet, because SSL
-	 * renegotiations might have required reading from the socket.
-	 */
+	/* retry after processing interrupts */
 	if (n < 0 && errno == EINTR)
 	{
+		/*
+		 * We tried to send data, the socket was full, and we were interrupted
+		 * while waiting for writability. We only process interrupts if we got
+		 * interrupted while writing and when in blocking mode. In other cases
+		 * it's better to allow the interrupts to be handled at higher layers.
+		 */
+		ProcessClientWriteInterrupt(!port->noblock);
+
 		goto retry;
 	}
 
@@ -262,17 +274,21 @@ wloop:
 		int		w;
 		int		save_errno = errno;
 
-		/*
-		 * We probably want to check for latches being set at some point
-		 * here. That'd allow us to handle interrupts while blocked on
-		 * writes. If set we'd not retry directly, but return. That way we
-		 * don't do anything while (possibly) inside a ssl library.
-		 */
 		w = WaitLatchOrSocket(MyLatch,
-							  WL_SOCKET_WRITEABLE,
+							  WL_LATCH_SET | WL_SOCKET_WRITEABLE,
 							  port->sock, 0);
 
-		if (w & WL_SOCKET_WRITEABLE)
+		if (w & WL_LATCH_SET)
+		{
+			ResetLatch(MyLatch);
+			/*
+			 * Force a return, so interrupts can be processed when not
+			 * (possibly) underneath a ssl library.
+			 */
+			errno = EINTR;
+			return -1;
+		}
+		else if (w & WL_SOCKET_WRITEABLE)
 		{
 			goto wloop;
 		}