Push enable/disable of notify and catchup interrupts all the way down

to just around the bare recv() call that gets a command from the client.
The former placement in PostgresMain was unsafe because the intermediate
processing layers (especially SSL) use facilities such as malloc that are
not necessarily re-entrant.  Per report from counterstorm.com.

3 files changed, 141 insertions, 22 deletions

diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c
index fbd8fe4d7fd..2962f0d2b51 100644
--- a/src/backend/libpq/be-secure.c
+++ b/src/backend/libpq/be-secure.c
@@ -11,7 +11,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/libpq/be-secure.c,v 1.15.2.12 2003/08/04 17:58:25 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/libpq/be-secure.c,v 1.15.2.13 2005/06/02 21:04:29 tgl Exp $
  *
  *	  Since the server static private key ($DataDir/server.key)
  *	  will normally be stored unencrypted so that the database
@@ -85,6 +85,8 @@
 #include "libpq/libpq.h"
 #include "libpq/pqsignal.h"
 #include "miscadmin.h"
+#include "tcop/tcopprot.h"
+
 
 #ifdef WIN32
 #include "win32.h"
@@ -308,8 +310,14 @@ secure_read(Port *port, void *ptr, size_t len)
 	}
 	else
 #endif
+	{
+		prepare_for_client_read();
+
 		n = recv(port->sock, ptr, len, 0);
 
+		client_read_ended();
+	}
+
 	return n;
 }
 
@@ -390,6 +398,73 @@ secure_write(Port *port, void *ptr, size_t len)
 /*						  SSL specific code						*/
 /* ------------------------------------------------------------ */
 #ifdef USE_SSL
+
+/*
+ * Private substitute BIO: this wraps the SSL library's standard socket BIO
+ * so that we can enable and disable interrupts just while calling recv().
+ * We cannot have interrupts occurring while the bulk of openssl runs,
+ * because it uses malloc() and possibly other non-reentrant libc facilities.
+ *
+ * As of openssl 0.9.7, we can use the reasonably clean method of interposing
+ * a wrapper around the standard socket BIO's sock_read() method.  This relies
+ * on the fact that sock_read() doesn't call anything non-reentrant, in fact
+ * not much of anything at all except recv().  If this ever changes we'd
+ * probably need to duplicate the code of sock_read() in order to push the
+ * interrupt enable/disable down yet another level.
+ */
+
+static bool my_bio_initialized = false;
+static BIO_METHOD my_bio_methods;
+static int (*std_sock_read) (BIO *h, char *buf, int size);
+
+static int
+my_sock_read(BIO *h, char *buf, int size)
+{
+	int		res;
+
+	prepare_for_client_read();
+
+	res = std_sock_read(h, buf, size);
+
+	client_read_ended();
+
+	return res;
+}
+
+static BIO_METHOD *
+my_BIO_s_socket(void)
+{
+	if (!my_bio_initialized)
+	{
+		memcpy(&my_bio_methods, BIO_s_socket(), sizeof(BIO_METHOD));
+		std_sock_read = my_bio_methods.bread;
+		my_bio_methods.bread = my_sock_read;
+		my_bio_initialized = true;
+	}
+	return &my_bio_methods;
+}
+
+/* This should exactly match openssl's SSL_set_fd except for using my BIO */
+static int
+my_SSL_set_fd(SSL *s, int fd)
+{
+	int ret=0;
+	BIO *bio=NULL;
+
+	bio=BIO_new(my_BIO_s_socket());
+
+	if (bio == NULL)
+	{
+		SSLerr(SSL_F_SSL_SET_FD,ERR_R_BUF_LIB);
+		goto err;
+	}
+	BIO_set_fd(bio,fd,BIO_NOCLOSE);
+	SSL_set_bio(s,bio,bio);
+	ret=1;
+err:
+	return(ret);
+}
+
 /*
  *	Load precomputed DH parameters.
  *
@@ -701,7 +776,7 @@ static int
 open_server_SSL(Port *port)
 {
 	if (!(port->ssl = SSL_new(SSL_context)) ||
-		!SSL_set_fd(port->ssl, port->sock) ||
+		!my_SSL_set_fd(port->ssl, port->sock) ||
 		SSL_accept(port->ssl) <= 0)
 	{
 		elog(COMMERROR, "failed to initialize SSL connection: %s", SSLerrmessage());
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index d756b6f2a6c..158b8ee8bcd 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.307.2.1 2003/01/01 21:57:18 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/tcop/postgres.c,v 1.307.2.2 2005/06/02 21:04:30 tgl Exp $
  *
  * NOTES
  *	  this is the "main" module of the postgres backend and
@@ -92,6 +92,13 @@ static bool EchoQuery = false;	/* default don't echo */
  */
 static volatile bool got_SIGHUP = false;
 
+/*
+ * Flag to indicate that we are doing the outer loop's read-from-client,
+ * as opposed to any random read from client that might happen within
+ * commands like COPY FROM STDIN.
+ */
+static bool DoingCommandRead = false;
+
 /* ----------------
  *		people who want to use EOF should #define DONTUSENEWLINE in
  *		tcop/tcopdebug.h
@@ -307,6 +314,50 @@ ReadCommand(StringInfo inBuf)
 	return result;
 }
 
+/*
+ * prepare_for_client_read -- set up to possibly block on client input
+ *
+ * This must be called immediately before any low-level read from the
+ * client connection.  It is necessary to do it at a sufficiently low level
+ * that there won't be any other operations except the read kernel call
+ * itself between this call and the subsequent client_read_ended() call.
+ * In particular there mustn't be use of malloc() or other potentially
+ * non-reentrant libc functions.  This restriction makes it safe for us
+ * to allow interrupt service routines to execute nontrivial code while
+ * we are waiting for input.
+ */
+void
+prepare_for_client_read(void)
+{
+	if (DoingCommandRead)
+	{
+		/* Enable immediate processing of asynchronous signals */
+		EnableNotifyInterrupt();
+
+		/* Allow "die" interrupt to be processed while waiting */
+		ImmediateInterruptOK = true;
+
+		/* And don't forget to detect one that already arrived */
+		QueryCancelPending = false;
+		CHECK_FOR_INTERRUPTS();
+	}
+}
+
+/*
+ * client_read_ended -- get out of the client-input state
+ */
+void
+client_read_ended(void)
+{
+	if (DoingCommandRead)
+	{
+		ImmediateInterruptOK = false;
+		QueryCancelPending = false;		/* forget any CANCEL signal */
+
+		DisableNotifyInterrupt();
+	}
+}
+
 
 /*
  * Parse a query string and pass it through the rewriter.
@@ -1781,7 +1832,7 @@ PostgresMain(int argc, char *argv[], const char *username)
 	if (!IsUnderPostmaster)
 	{
 		puts("\nPOSTGRES backend interactive interface ");
-		puts("$Revision: 1.307.2.1 $ $Date: 2003/01/01 21:57:18 $\n");
+		puts("$Revision: 1.307.2.2 $ $Date: 2005/06/02 21:04:30 $\n");
 	}
 
 	/*
@@ -1830,6 +1881,7 @@ PostgresMain(int argc, char *argv[], const char *username)
 		CritSectionCount = 0;	/* should be unnecessary, but... */
 		disable_sig_alarm(true);
 		QueryCancelPending = false;	/* again in case timeout occurred */
+		DoingCommandRead = false;
 		DisableNotifyInterrupt();
 		debug_query_string = NULL;
 
@@ -1909,20 +1961,13 @@ PostgresMain(int argc, char *argv[], const char *username)
 		}
 
 		/*
-		 * (2) deal with pending asynchronous NOTIFY from other backends,
-		 * and enable async.c's signal handler to execute NOTIFY directly.
-		 * Then set up other stuff needed before blocking for input.
+		 * (2) Allow asynchronous signals to be executed immediately
+		 * if they come in while we are waiting for client input.
+		 * (This must be conditional since we don't want, say, reads on
+		 * behalf of COPY FROM STDIN doing the same thing.)
 		 */
-		QueryCancelPending = false;		/* forget any earlier CANCEL
-										 * signal */
-
-		EnableNotifyInterrupt();
-
-		/* Allow "die" interrupt to be processed while waiting */
-		ImmediateInterruptOK = true;
-		/* and don't forget to detect one that already arrived */
-		QueryCancelPending = false;
-		CHECK_FOR_INTERRUPTS();
+		QueryCancelPending = false;		/* forget any earlier CANCEL signal */
+		DoingCommandRead = true;
 
 		/*
 		 * (3) read a command (loop blocks here)
@@ -1932,10 +1977,7 @@ PostgresMain(int argc, char *argv[], const char *username)
 		/*
 		 * (4) disable async signal conditions again.
 		 */
-		ImmediateInterruptOK = false;
-		QueryCancelPending = false;		/* forget any CANCEL signal */
-
-		DisableNotifyInterrupt();
+		DoingCommandRead = false;
 
 		/*
 		 * (5) check for any other interesting events that happened while
diff --git a/src/include/tcop/tcopprot.h b/src/include/tcop/tcopprot.h
index f4ba67b3be0..ce7df5286f6 100644
--- a/src/include/tcop/tcopprot.h
+++ b/src/include/tcop/tcopprot.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: tcopprot.h,v 1.51 2002/10/21 18:57:35 petere Exp $
+ * $Id: tcopprot.h,v 1.51.2.1 2005/06/02 21:04:30 tgl Exp $
  *
  * OLD COMMENTS
  *	  This file was created so that other c files could get the two
@@ -50,6 +50,8 @@ extern void pg_exec_query_string(StringInfo query_string,
 extern void die(SIGNAL_ARGS);
 extern void quickdie(SIGNAL_ARGS);
 extern void authdie(SIGNAL_ARGS);
+extern void prepare_for_client_read(void);
+extern void client_read_ended(void);
 extern int	PostgresMain(int argc, char *argv[], const char *username);
 extern void ResetUsage(void);
 extern void ShowUsage(const char *title);