Repair bug that would allow libpq to think a command had succeeded when

it really hadn't, due to double output of previous command's response.
Fix prevents recursive entry to libpq routines.  Found by Jan Wieck.

3 files changed, 94 insertions, 22 deletions

diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c
index f0778e951e4..d6b27985560 100644
--- a/src/backend/libpq/pqcomm.c
+++ b/src/backend/libpq/pqcomm.c
@@ -30,7 +30,7 @@
  * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- *	$Header: /cvsroot/pgsql/src/backend/libpq/pqcomm.c,v 1.166 2003/09/25 06:57:59 petere Exp $
+ *	$Header: /cvsroot/pgsql/src/backend/libpq/pqcomm.c,v 1.166.2.1 2004/09/26 00:26:50 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -44,6 +44,7 @@
  *		StreamClose			- Close a client/backend connection
  *		TouchSocketFile		- Protect socket file against /tmp cleaners
  *		pq_init			- initialize libpq at backend startup
+ *		pq_comm_reset	- reset libpq during error recovery
  *		pq_close		- shutdown libpq at backend exit
  *
  * low-level I/O:
@@ -88,14 +89,6 @@
 #include "storage/ipc.h"
 
 
-static void pq_close(void);
-
-#ifdef HAVE_UNIX_SOCKETS
-static int	Lock_AF_UNIX(unsigned short portNumber, char *unixSocketName);
-static int	Setup_AF_UNIX(void);
-#endif   /* HAVE_UNIX_SOCKETS */
-
-
 /*
  * Configuration options
  */
@@ -103,6 +96,10 @@ int			Unix_socket_permissions;
 char	   *Unix_socket_group;
 
 
+/* Where the Unix socket file is */
+static char sock_path[MAXPGPATH];
+
+
 /*
  * Buffers for low-level I/O
  */
@@ -121,9 +118,20 @@ static int	PqRecvLength;		/* End of data available in PqRecvBuffer */
 /*
  * Message status
  */
+static bool PqCommBusy;
 static bool DoingCopyOut;
 
 
+/* Internal functions */
+static void pq_close(void);
+static int	internal_putbytes(const char *s, size_t len);
+static int	internal_flush(void);
+#ifdef HAVE_UNIX_SOCKETS
+static int	Lock_AF_UNIX(unsigned short portNumber, char *unixSocketName);
+static int	Setup_AF_UNIX(void);
+#endif   /* HAVE_UNIX_SOCKETS */
+
+
 /* --------------------------------
  *		pq_init - initialize libpq at backend startup
  * --------------------------------
@@ -132,10 +140,27 @@ void
 pq_init(void)
 {
 	PqSendPointer = PqRecvPointer = PqRecvLength = 0;
+	PqCommBusy = false;
 	DoingCopyOut = false;
 	on_proc_exit(pq_close, 0);
 }
 
+/* --------------------------------
+ *		pq_comm_reset - reset libpq during error recovery
+ *
+ * This is called from error recovery at the outer idle loop.  It's
+ * just to get us out of trouble if we somehow manage to elog() from
+ * inside a pqcomm.c routine (which ideally will never happen, but...)
+ * --------------------------------
+ */
+void
+pq_comm_reset(void)
+{
+	/* Do not throw away pending data, but do reset the busy flag */
+	PqCommBusy = false;
+	/* We can abort any old-style COPY OUT, too */
+	pq_endcopyout(true);
+}
 
 /* --------------------------------
  *		pq_close - shutdown libpq at backend exit
@@ -174,8 +199,6 @@ pq_close(void)
  *		Stream functions are used for vanilla TCP connection protocol.
  */
 
-static char sock_path[MAXPGPATH];
-
 
 /* StreamDoUnlink()
  * Shutdown routine for backend connection
@@ -885,12 +908,29 @@ pq_getmessage(StringInfo s, int maxlen)
 int
 pq_putbytes(const char *s, size_t len)
 {
+	int			res;
+
+	/* Should only be called by old-style COPY OUT */
+	Assert(DoingCopyOut);
+	/* No-op if reentrant call */
+	if (PqCommBusy)
+		return 0;
+	PqCommBusy = true;
+	res = internal_putbytes(s, len);
+	PqCommBusy = false;
+	return res;
+}
+
+static int
+internal_putbytes(const char *s, size_t len)
+{
 	size_t		amount;
 
 	while (len > 0)
 	{
+		/* If buffer is full, then flush it out */
 		if (PqSendPointer >= PQ_BUFFER_SIZE)
-			if (pq_flush())		/* If buffer is full, then flush it out */
+			if (internal_flush())
 				return EOF;
 		amount = PQ_BUFFER_SIZE - PqSendPointer;
 		if (amount > len)
@@ -912,6 +952,20 @@ pq_putbytes(const char *s, size_t len)
 int
 pq_flush(void)
 {
+	int			res;
+
+	/* No-op if reentrant call */
+	if (PqCommBusy)
+		return 0;
+	PqCommBusy = true;
+	res = internal_flush();
+	PqCommBusy = false;
+	return res;
+}
+
+static int
+internal_flush(void)
+{
 	static int	last_reported_send_errno = 0;
 
 	unsigned char *bufptr = PqSendBuffer;
@@ -987,26 +1041,40 @@ pq_flush(void)
  *		then; dropping them is annoying, but at least they will still appear
  *		in the postmaster log.)
  *
+ *		We also suppress messages generated while pqcomm.c is busy.  This
+ *		avoids any possibility of messages being inserted within other
+ *		messages.  The only known trouble case arises if SIGQUIT occurs
+ *		during a pqcomm.c routine --- quickdie() will try to send a warning
+ *		message, and the most reasonable approach seems to be to drop it.
+ *
  *		returns 0 if OK, EOF if trouble
  * --------------------------------
  */
 int
 pq_putmessage(char msgtype, const char *s, size_t len)
 {
-	if (DoingCopyOut)
+	if (DoingCopyOut || PqCommBusy)
 		return 0;
+	PqCommBusy = true;
 	if (msgtype)
-		if (pq_putbytes(&msgtype, 1))
-			return EOF;
+		if (internal_putbytes(&msgtype, 1))
+			goto fail;
 	if (PG_PROTOCOL_MAJOR(FrontendProtocol) >= 3)
 	{
 		uint32		n32;
 
 		n32 = htonl((uint32) (len + 4));
-		if (pq_putbytes((char *) &n32, 4))
-			return EOF;
+		if (internal_putbytes((char *) &n32, 4))
+			goto fail;
 	}
-	return pq_putbytes(s, len);
+	if (internal_putbytes(s, len))
+		goto fail;
+	PqCommBusy = false;
+	return 0;
+
+fail:
+	PqCommBusy = false;
+	return EOF;
 }
 
 /* --------------------------------
@@ -1035,8 +1103,8 @@ pq_endcopyout(bool errorAbort)
 {
 	if (!DoingCopyOut)
 		return;
-	DoingCopyOut = false;
 	if (errorAbort)
 		pq_putbytes("\n\n\\.\n", 5);
 	/* in non-error case, copy.c will have emitted the terminator line */
+	DoingCopyOut = false;
 }
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 23e8c75e166..b091f5d3dbd 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/tcop/postgres.c,v 1.375.2.1 2003/11/24 14:50:02 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/tcop/postgres.c,v 1.375.2.2 2004/09/26 00:26:53 tgl Exp $
  *
  * NOTES
  *	  this is the "main" module of the postgres backend and
@@ -2659,7 +2659,7 @@ PostgresMain(int argc, char *argv[], const char *username)
 	if (!IsUnderPostmaster)
 	{
 		puts("\nPOSTGRES backend interactive interface ");
-		puts("$Revision: 1.375.2.1 $ $Date: 2003/11/24 14:50:02 $\n");
+		puts("$Revision: 1.375.2.2 $ $Date: 2004/09/26 00:26:53 $\n");
 	}
 
 	/*
@@ -2709,6 +2709,9 @@ PostgresMain(int argc, char *argv[], const char *username)
 		DisableNotifyInterrupt();
 		debug_query_string = NULL;
 
+		/* Make sure libpq is in a good state */
+		pq_comm_reset();
+
 		/*
 		 * Make sure we are in a valid memory context during recovery.
 		 *
diff --git a/src/include/libpq/libpq.h b/src/include/libpq/libpq.h
index 4bd7f40a583..d50f6ca14c9 100644
--- a/src/include/libpq/libpq.h
+++ b/src/include/libpq/libpq.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: libpq.h,v 1.60 2003/08/04 02:40:13 momjian Exp $
+ * $Id: libpq.h,v 1.60.4.1 2004/09/26 00:26:56 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -52,6 +52,7 @@ extern int	StreamConnection(int server_fd, Port *port);
 extern void StreamClose(int sock);
 extern void TouchSocketFile(void);
 extern void pq_init(void);
+extern void pq_comm_reset(void);
 extern int	pq_getbytes(char *s, size_t len);
 extern int	pq_getstring(StringInfo s);
 extern int	pq_getmessage(StringInfo s, int maxlen);