Fix pq_getbyte_if_available() function. It was confused on what it

returns if no data is immediately available. Patch by me with numerous
fixes from Fujii Masao and Magnus Hagander.

3 files changed, 49 insertions, 38 deletions

diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c
index ea6b16c13ba..94998999501 100644
--- a/src/backend/libpq/be-secure.c
+++ b/src/backend/libpq/be-secure.c
@@ -11,7 +11,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/libpq/be-secure.c,v 1.96 2010/01/15 09:19:02 heikki Exp $
+ *	  $PostgreSQL: pgsql/src/backend/libpq/be-secure.c,v 1.97 2010/02/18 11:13:45 heikki Exp $
  *
  *	  Since the server static private key ($DataDir/server.key)
  *	  will normally be stored unencrypted so that the database
@@ -256,7 +256,11 @@ rloop:
 			case SSL_ERROR_WANT_READ:
 			case SSL_ERROR_WANT_WRITE:
 				if (port->noblock)
-					return 0;
+				{
+					errno = EWOULDBLOCK;
+					n = -1;
+					break;
+				}
 #ifdef WIN32
 				pgwin32_waitforsinglesocket(SSL_get_fd(port->ssl),
 											(err == SSL_ERROR_WANT_READ) ?
diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c
index 8f40c93d892..98b27f1f71a 100644
--- a/src/backend/libpq/pqcomm.c
+++ b/src/backend/libpq/pqcomm.c
@@ -30,7 +30,7 @@
  * Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- *	$PostgreSQL: pgsql/src/backend/libpq/pqcomm.c,v 1.203 2010/02/16 19:26:02 mha Exp $
+ *	$PostgreSQL: pgsql/src/backend/libpq/pqcomm.c,v 1.204 2010/02/18 11:13:45 heikki Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -821,8 +821,8 @@ pq_peekbyte(void)
  *		pq_getbyte_if_available	- get a single byte from connection,
  *			if available
  *
- * The received byte is stored in *c. Returns 1 if a byte was read, 0 if
- * if no data was available, or EOF.
+ * The received byte is stored in *c. Returns 1 if a byte was read,
+ * 0 if no data was available, or EOF if trouble.
  * --------------------------------
  */
 int
@@ -848,6 +848,33 @@ pq_getbyte_if_available(unsigned char *c)
 	PG_TRY();
 	{
 		r = secure_read(MyProcPort, c, 1);
+		if (r < 0)
+		{
+			/*
+			 * Ok if no data available without blocking or interrupted
+			 * (though EINTR really shouldn't happen with a non-blocking
+			 * socket). Report other errors.
+			 */
+			if (errno == EAGAIN || errno == EWOULDBLOCK || errno == EINTR)
+				r = 0;
+			else
+			{
+				/*
+				 * Careful: an ereport() that tries to write to the client would
+				 * cause recursion to here, leading to stack overflow and core
+				 * dump!  This message must go *only* to the postmaster log.
+				 */
+				ereport(COMMERROR,
+						(errcode_for_socket_access(),
+						 errmsg("could not receive data from client: %m")));
+				r = EOF;
+			}
+		}
+		else if (r == 0)
+		{
+			/* EOF detected */
+			r = EOF;
+		}
 	}
 	PG_CATCH();
 	{
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index eb9a5425a2e..bf533fff73d 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -30,7 +30,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/replication/walsender.c,v 1.6 2010/02/17 04:19:39 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/replication/walsender.c,v 1.7 2010/02/18 11:13:46 heikki Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -176,14 +176,7 @@ WalSndHandshake(void)
 			ProcessConfigFile(PGC_SIGHUP);
 		}
 
-		if (firstchar == EOF)
-		{
-			/* standby disconnected */
-			ereport(COMMERROR,
-					(errcode(ERRCODE_PROTOCOL_VIOLATION),
-					 errmsg("unexpected EOF on standby connection")));
-		}
-		else
+		if (firstchar != EOF)
 		{
 			/*
 			 * Read the message contents. This is expected to be done without
@@ -193,9 +186,7 @@ WalSndHandshake(void)
 				firstchar = EOF;		/* suitable message already logged */
 		}
 
-
 		/* Handle the very limited subset of commands expected in this phase */
-
 		switch (firstchar)
 		{
 			case 'Q':	/* Query message */
@@ -286,14 +277,16 @@ WalSndHandshake(void)
 				break;
 			}
 
-			/* 'X' means that the standby is closing the connection */
 			case 'X':
+				/* standby is closing the connection */
 				proc_exit(0);
 
 			case EOF:
-				ereport(ERROR,
+				/* standby disconnected unexpectedly */
+				ereport(COMMERROR,
 						(errcode(ERRCODE_PROTOCOL_VIOLATION),
 						 errmsg("unexpected EOF on standby connection")));
+				proc_exit(0);
 
 			default:
 				ereport(FATAL,
@@ -315,36 +308,23 @@ CheckClosedConnection(void)
 	r = pq_getbyte_if_available(&firstchar);
 	if (r < 0)
 	{
-		/* no data available */
-		if (errno == EAGAIN || errno == EWOULDBLOCK)
-			return;
-
-		/*
-		 * Ok if interrupted, though it shouldn't really happen with
-		 * a non-blocking operation.
-		 */
-		if (errno == EINTR)
-			return;
-
+		/* unexpected error or EOF */
 		ereport(COMMERROR,
-				(errcode_for_socket_access(),
-				 errmsg("could not receive data from client: %m")));
+				(errcode(ERRCODE_PROTOCOL_VIOLATION),
+				 errmsg("unexpected EOF on standby connection")));
+		proc_exit(0);
 	}
 	if (r == 0)
 	{
-		/* standby disconnected unexpectedly */
-		ereport(ERROR,
-				(errcode(ERRCODE_PROTOCOL_VIOLATION),
-				 errmsg("unexpected EOF on standby connection")));
+		/* no data available without blocking */
+		return;
 	}
 
 	/* Handle the very limited subset of commands expected in this phase */
 	switch (firstchar)
 	{
 		/*
-		 * 'X' means that the standby is closing down the socket. EOF means
-		 * unexpected loss of standby connection. Either way, perform normal
-		 * shutdown.
+		 * 'X' means that the standby is closing down the socket.
 		 */
 		case 'X':
 			proc_exit(0);