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.

diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c
index a8ce982bdb96e5628c88d7a65128fdcfa428dafa..805577bd77471f237ac246e0befbd2aa2b2fe5ad 100644 (file)
--- a/src/backend/libpq/pqcomm.c
+++ b/src/backend/libpq/pqcomm.c
@@ -30,7 +30,7 @@
  * Portions Copyright (c) 1996-2004, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- *     $PostgreSQL: pgsql/src/backend/libpq/pqcomm.c,v 1.171 2004/08/29 05:06:43 momjian Exp $
+ *     $PostgreSQL: pgsql/src/backend/libpq/pqcomm.c,v 1.172 2004/09/26 00:26:19 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(int code, Datum arg);
-
-#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(int code, Datum arg);
+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(int code, Datum arg)
  *             Stream functions are used for vanilla TCP connection protocol.
  */
 
-static char sock_path[MAXPGPATH];
-
 
 /* StreamDoUnlink()
  * Shutdown routine for backend connection
@@ -885,13 +908,30 @@ 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;
 
@@ -988,26 +1042,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;
 }
 
 /* --------------------------------
@@ -1036,8 +1104,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 85fcc49c839c870d2579842151f541f9138a5fcf..5658e10d4533183d5197f202e6cd073a2784adcf 100644 (file)
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.432 2004/09/13 20:07:05 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.433 2004/09/26 00:26:25 tgl Exp $
  *
  * NOTES
  *       this is the "main" module of the postgres backend and
@@ -2811,6 +2811,9 @@ PostgresMain(int argc, char *argv[], const char *username)
                DisableNotifyInterrupt();
                DisableCatchupInterrupt();
 
+               /* Make sure libpq is in a good state */
+               pq_comm_reset();
+
                /* Report the error to the client and/or server log */
                EmitErrorReport();
 

diff --git a/src/include/libpq/libpq.h b/src/include/libpq/libpq.h
index bf58f75ac28b17300b7aacb798d33ae63bf359ce..839d665d3c7d5b7699417c6e62a6ce0191a75b3c 100644 (file)
--- a/src/include/libpq/libpq.h
+++ b/src/include/libpq/libpq.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2004, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/libpq/libpq.h,v 1.62 2004/08/29 04:13:07 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/libpq/libpq.h,v 1.63 2004/09/26 00:26:28 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);