Rework libpq threaded SIGPIPE handling to avoid interference with

calling applications.  This is done by blocking sigpipe in the libpq
thread and using sigpending/sigwait to possibily discard any sigpipe we
generated.

Author: bmomjian

configure

@@ -17431,6 +17431,18 @@ _ACEOF
 fi
 HAVE_POSIX_SIGNALS=$pgac_cv_func_posix_signals
 
+if test "$pgac_cv_func_posix_signals" != yes -a "$enable_thread_safety" = yes; then
+  { { echo "$as_me:$LINENO: error:
+*** Thread-safety requires POSIX signals, which are not supported by your
+*** operating system.
+" >&5
+echo "$as_me: error:
+*** Thread-safety requires POSIX signals, which are not supported by your
+*** operating system.
+" >&2;}
+   { (exit 1); exit 1; }; }
+fi
+
 if test $ac_cv_func_fseeko = yes; then
 # Check whether --enable-largefile or --disable-largefile was given.
 if test "${enable_largefile+set}" = set; then

configure.in

@@ -1,5 +1,5 @@
 dnl Process this file with autoconf to produce a configure script.
-dnl $PostgreSQL: pgsql/configure.in,v 1.387 2004/11/30 06:13:04 tgl Exp $
+dnl $PostgreSQL: pgsql/configure.in,v 1.388 2004/12/02 15:32:50 momjian Exp $
 dnl
 dnl Developers, please strive to achieve this order:
 dnl
@@ -1174,6 +1174,13 @@ AC_CHECK_TYPES(sig_atomic_t, [], [], [#include <signal.h>])
 
 
 PGAC_FUNC_POSIX_SIGNALS
+if test "$pgac_cv_func_posix_signals" != yes -a "$enable_thread_safety" = yes; then
+  AC_MSG_ERROR([
+*** Thread-safety requires POSIX signals, which are not supported by your
+*** operating system.
+])
+fi
+
 if test $ac_cv_func_fseeko = yes; then
 AC_SYS_LARGEFILE
 fi

doc/src/sgml/libpq.sgml

@@ -1,5 +1,5 @@
 <!--
-$PostgreSQL: pgsql/doc/src/sgml/libpq.sgml,v 1.169 2004/11/27 21:56:04 petere Exp $
+$PostgreSQL: pgsql/doc/src/sgml/libpq.sgml,v 1.170 2004/12/02 15:32:52 momjian Exp $
 -->
 
  <chapter id="libpq">
@@ -3954,24 +3954,6 @@ safety</></> It is better to use the <literal>md5</literal> method,
 which is thread-safe on all platforms.
 </para>
 
-<para>
-<application>libpq</application> must ignore <literal>SIGPIPE</> signals
-generated internally by <function>send()</> calls to backend processes.
-When <productname>PostgreSQL</> is configured without
-<literal>--enable-thread-safety</>, <application>libpq</> sets
-<literal>SIGPIPE</> to <literal>SIG_IGN</> before each
-<function>send()</> call and restores the original signal handler after
-completion. When <literal>--enable-thread-safety</> is used,
-<application>libpq</> installs its own <literal>SIGPIPE</> handler
-before the first database connection.  This handler uses thread-local
-storage to determine if a <literal>SIGPIPE</> signal has been generated
-by a libpq <function>send()</>. If an application wants to install
-its own <literal>SIGPIPE</> signal handler, it should call
-<function>PQinSend()</> to determine if it should ignore the
-<literal>SIGPIPE</> signal. This function is available in both
-thread-safe and non-thread-safe versions of <application>libpq</>.
-</para>
-
 <para>
 If you experience problems with threaded applications, run
 the program in <filename>src/tools/thread</> to see if your

doc/src/sgml/ref/copy.sgml

@@ -1,8 +1,9 @@
 <!--
-$PostgreSQL: pgsql/doc/src/sgml/ref/copy.sgml,v 1.60 2004/11/27 21:56:05 petere Exp $
+$PostgreSQL: pgsql/doc/src/sgml/ref/copy.sgml,v 1.61 2004/12/02 15:32:53 momjian Exp $
 PostgreSQL documentation
 -->
 
+
 <refentry id="SQL-COPY">
  <refmeta>
   <refentrytitle id="sql-copy-title">COPY</refentrytitle>

src/interfaces/libpq/fe-connect.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/interfaces/libpq/fe-connect.c,v 1.290 2004/12/01 23:42:26 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/interfaces/libpq/fe-connect.c,v 1.291 2004/12/02 15:32:54 momjian Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -866,15 +866,6 @@ connectDBStart(PGconn *conn)
 	const char *node = NULL;
 	int			ret;
 
-#ifdef ENABLE_THREAD_SAFETY
-#ifndef WIN32
-	static pthread_once_t check_sigpipe_once = PTHREAD_ONCE_INIT;
-
-	/* Check only on first connection request */
-	pthread_once(&check_sigpipe_once, pq_check_sigpipe_handler);
-#endif
-#endif
-
 	if (!conn)
 		return 0;
 

src/interfaces/libpq/fe-print.c

@@ -10,7 +10,7 @@
  * didn't really belong there.
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/interfaces/libpq/fe-print.c,v 1.55 2004/11/09 15:57:57 petere Exp $
+ *	  $PostgreSQL: pgsql/src/interfaces/libpq/fe-print.c,v 1.56 2004/12/02 15:32:54 momjian Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -91,7 +91,11 @@ PQprint(FILE *fout,
 		int			total_line_length = 0;
 		int			usePipe = 0;
 		char	   *pagerenv;
-
+#ifdef ENABLE_THREAD_SAFETY
+		sigset_t	osigset;
+		bool		sigpipe_masked = false;
+		bool		sigpipe_pending;
+#endif
 #if !defined(ENABLE_THREAD_SAFETY) && !defined(WIN32)
 		pqsigfunc	oldsigpipehandler = NULL;
 #endif
@@ -189,7 +193,8 @@ PQprint(FILE *fout,
 				{
 					usePipe = 1;
 #ifdef ENABLE_THREAD_SAFETY
-					pthread_setspecific(pq_thread_in_send, "t");
+					pq_block_sigpipe(&osigset, &sigpipe_pending);
+					sigpipe_masked = true;
 #else
 #ifndef WIN32
 					oldsigpipehandler = pqsignal(SIGPIPE, SIG_IGN);
@@ -311,7 +316,8 @@ PQprint(FILE *fout,
 			pclose(fout);
 #endif
 #ifdef ENABLE_THREAD_SAFETY
-			pthread_setspecific(pq_thread_in_send, "f");
+			if (sigpipe_masked)
+				pq_reset_sigpipe(&osigset, sigpipe_pending);
 #else
 #ifndef WIN32
 			pqsignal(SIGPIPE, oldsigpipehandler);

src/interfaces/libpq/fe-secure.c

@@ -11,7 +11,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/interfaces/libpq/fe-secure.c,v 1.57 2004/11/20 00:35:13 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/interfaces/libpq/fe-secure.c,v 1.58 2004/12/02 15:32:54 momjian Exp $
  *
  * NOTES
  *	  [ Most of these notes are wrong/obsolete, but perhaps not all ]
@@ -152,12 +152,6 @@ bool		pq_initssllib = true;
 static SSL_CTX *SSL_context = NULL;
 #endif
 
-#ifdef ENABLE_THREAD_SAFETY
-static void sigpipe_handler_ignore_send(int signo);
-pthread_key_t pq_thread_in_send = 0;	/* initializer needed on Darwin */
-static pqsigfunc pq_pipe_handler;
-#endif
-
 /* ------------------------------------------------------------ */
 /*						 Hardcoded values						*/
 /* ------------------------------------------------------------ */
@@ -379,9 +373,12 @@ ssize_t
 pqsecure_write(PGconn *conn, const void *ptr, size_t len)
 {
 	ssize_t		n;
-
+	
 #ifdef ENABLE_THREAD_SAFETY
-	pthread_setspecific(pq_thread_in_send, "t");
+	sigset_t	osigmask;
+	bool		sigpipe_pending;
+	
+	pq_block_sigpipe(&osigmask, &sigpipe_pending);
 #else
 #ifndef WIN32
 	pqsigfunc	oldsighandler = pqsignal(SIGPIPE, SIG_IGN);
@@ -452,9 +449,14 @@ pqsecure_write(PGconn *conn, const void *ptr, size_t len)
 	else
 #endif
 		n = send(conn->sock, ptr, len, 0);
+		/*
+		 *	Possible optimization:  if sigpending() turns out to be an
+		 *	expensive operation, we can set sigpipe_pending = 'true'
+		 *	here if errno != EPIPE, avoiding a sigpending call.
+		 */
 
 #ifdef ENABLE_THREAD_SAFETY
-	pthread_setspecific(pq_thread_in_send, "f");
+	pq_reset_sigpipe(&osigmask, sigpipe_pending);
 #else
 #ifndef WIN32
 	pqsignal(SIGPIPE, oldsighandler);
@@ -1216,65 +1218,77 @@ PQgetssl(PGconn *conn)
 }
 #endif   /* USE_SSL */
 
-
 #ifdef ENABLE_THREAD_SAFETY
-#ifndef WIN32
 /*
- *	Check SIGPIPE handler and perhaps install our own.
+ *	Block SIGPIPE for this thread.  This prevents send()/write() from exiting
+ *	the application.
  */
-void
-pq_check_sigpipe_handler(void)
-{
-	pthread_key_create(&pq_thread_in_send, NULL);
-
-	/*
-	 * Find current pipe handler and chain on to it.
-	 */
-	pq_pipe_handler = pqsignalinquire(SIGPIPE);
-	pqsignal(SIGPIPE, sigpipe_handler_ignore_send);
-}
-
-/*
- *	Threaded SIGPIPE signal handler
- */
-void
-sigpipe_handler_ignore_send(int signo)
+int
+pq_block_sigpipe(sigset_t *osigset, bool *sigpipe_pending)
 {
-	/*
-	 * If we have gotten a SIGPIPE outside send(), chain or exit if we are
-	 * at the end of the chain. Synchronous signals are delivered to the
-	 * thread that caused the signal.
-	 */
-	if (!PQinSend())
+	sigset_t sigpipe_sigset;
+	sigset_t sigset;
+	int		 ret;
+	
+	sigemptyset(&sigpipe_sigset);
+	sigaddset(&sigpipe_sigset, SIGPIPE);
+
+	/* Block SIGPIPE and save previous mask for later reset */
+	ret = pthread_sigmask(SIG_BLOCK, &sigpipe_sigset, osigset);
+
+	/* We can have a pending SIGPIPE only if it was blocked before */
+	if (sigismember(osigset, SIGPIPE))
 	{
-		if (pq_pipe_handler == SIG_DFL) /* not set by application */
-			exit(128 + SIGPIPE);	/* typical return value for SIG_DFL */
+		/* Is there a pending SIGPIPE? */
+		if (sigpending(&sigset) != 0)
+			return -1;
+	
+		if (sigismember(&sigset, SIGPIPE))
+			*sigpipe_pending = true;
 		else
-			(*pq_pipe_handler) (signo); /* call original handler */
+			*sigpipe_pending = false;
 	}
+	else
+		*sigpipe_pending = false;
+	
+	return ret;
 }
-#endif
-#endif
-
+	
 /*
- *	Indicates whether the current thread is in send()
- *	For use by SIGPIPE signal handlers;  they should
- *	ignore SIGPIPE when libpq is in send().  This means
- *	that the backend has died unexpectedly.
+ *	Discard any pending SIGPIPE and reset the signal mask.
+ *	We might be discarding a blocked SIGPIPE that we didn't generate,
+ *	but we document that you can't keep blocked SIGPIPE calls across
+ *	libpq function calls.
  */
-pqbool
-PQinSend(void)
+int
+pq_reset_sigpipe(sigset_t *osigset, bool sigpipe_pending)
 {
-#ifdef ENABLE_THREAD_SAFETY
-	return (pthread_getspecific(pq_thread_in_send) /* has it been set? */ &&
-			*(char *) pthread_getspecific(pq_thread_in_send) == 't') ? true : false;
-#else
+	int	signo;
+	sigset_t sigset;
 
-	/*
-	 * No threading: our code ignores SIGPIPE around send(). Therefore, we
-	 * can't be in send() if we are checking from a SIGPIPE signal
-	 * handler.
-	 */
-	return false;
-#endif
+	/* Clear SIGPIPE only if none was pending */
+	if (!sigpipe_pending)
+	{
+		if (sigpending(&sigset) != 0)
+			return -1;
+	
+		/*
+		 *	Discard pending and blocked SIGPIPE
+		 */
+		if (sigismember(&sigset, SIGPIPE))
+		{
+			sigset_t sigpipe_sigset;
+			
+			sigemptyset(&sigpipe_sigset);
+			sigaddset(&sigpipe_sigset, SIGPIPE);
+	
+			sigwait(&sigpipe_sigset, &signo);
+			if (signo != SIGPIPE)
+				return -1;
+		}
+	}
+	
+	/* Restore saved block mask */
+	return pthread_sigmask(SIG_SETMASK, osigset, NULL);
 }
+#endif

src/interfaces/libpq/libpq-fe.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/interfaces/libpq/libpq-fe.h,v 1.113 2004/10/30 23:11:27 tgl Exp $
+ * $PostgreSQL: pgsql/src/interfaces/libpq/libpq-fe.h,v 1.114 2004/12/02 15:32:54 momjian Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -497,12 +497,6 @@ extern int	PQenv2encoding(void);
 
 /* === in fe-secure.c === */
 
-/*
- *	Indicates whether the libpq thread is in send().
- *	Used to ignore SIGPIPE if thread is in send().
- */
-extern pqbool PQinSend(void);
-
 #ifdef __cplusplus
 }
 #endif

src/interfaces/libpq/libpq-int.h

@@ -12,7 +12,7 @@
  * Portions Copyright (c) 1996-2004, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/interfaces/libpq/libpq-int.h,v 1.96 2004/10/30 23:11:27 tgl Exp $
+ * $PostgreSQL: pgsql/src/interfaces/libpq/libpq-int.h,v 1.97 2004/12/02 15:32:54 momjian Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -31,6 +31,7 @@
 
 #ifdef ENABLE_THREAD_SAFETY
 #include <pthread.h>
+#include <signal.h>
 #endif
 
 #ifdef WIN32_CLIENT_ONLY
@@ -475,15 +476,15 @@ extern void pqsecure_close(PGconn *);
 extern ssize_t pqsecure_read(PGconn *, void *ptr, size_t len);
 extern ssize_t pqsecure_write(PGconn *, const void *ptr, size_t len);
 
-#ifdef ENABLE_THREAD_SAFETY
-extern void pq_check_sigpipe_handler(void);
-extern pthread_key_t pq_thread_in_send;
-#endif
-
 #ifdef USE_SSL
 extern bool pq_initssllib;
 #endif
 
+#ifdef ENABLE_THREAD_SAFETY
+int pq_block_sigpipe(sigset_t *osigset, bool *sigpipe_pending);
+int pq_reset_sigpipe(sigset_t *osigset, bool sigpipe_pending);
+#endif
+
 /*
  * this is so that we can check if a connection is non-blocking internally
  * without the overhead of a function call