From 09a893117a9fe0ae5e7e5c6a6a5496e9e6826f4c Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sun, 26 Sep 2004 00:26:28 +0000
Subject: [PATCH] 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.
---
src/backend/libpq/pqcomm.c | 106 +++++++++++++++++++++++++++++-------
src/backend/tcop/postgres.c | 5 +-
src/include/libpq/libpq.h | 3 +-
3 files changed, 93 insertions(+), 21 deletions(-)
diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c
index a8ce982bdb9..805577bd774 100644
--- 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 85fcc49c839..5658e10d453 100644
--- 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 bf58f75ac28..839d665d3c7 100644
--- 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);
--
GitLab