From 8408f65252c23040e766b6a3ecde7bff390e2847 Mon Sep 17 00:00:00 2001
From: Bruce Momjian <bruce@momjian.us>
Date: Thu, 2 Dec 2004 15:32:54 +0000
Subject: [PATCH] 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.
---
configure | 12 +++
configure.in | 9 +-
doc/src/sgml/libpq.sgml | 20 +----
doc/src/sgml/ref/copy.sgml | 3 +-
src/interfaces/libpq/fe-connect.c | 11 +--
src/interfaces/libpq/fe-print.c | 14 +++-
src/interfaces/libpq/fe-secure.c | 132 +++++++++++++++++-------------
src/interfaces/libpq/libpq-fe.h | 8 +-
src/interfaces/libpq/libpq-int.h | 13 +--
9 files changed, 115 insertions(+), 107 deletions(-)
diff --git a/configure b/configure
index 009714367fc..378cd612910 100755
--- a/configure
+++ b/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
diff --git a/configure.in b/configure.in
index ed65d5fec1e..cbb2279ffec 100644
--- a/configure.in
+++ b/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
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index c47c33d56e2..cfcc7c96562 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/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
diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml
index d9a0130333f..96e0caecb87 100644
--- a/doc/src/sgml/ref/copy.sgml
+++ b/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>
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 6da2c79af4a..30e56b4b6eb 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/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;
diff --git a/src/interfaces/libpq/fe-print.c b/src/interfaces/libpq/fe-print.c
index 832af2b3ac0..9992de533d7 100644
--- a/src/interfaces/libpq/fe-print.c
+++ b/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);
diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
index 6b8d98001d6..79ea192f99c 100644
--- a/src/interfaces/libpq/fe-secure.c
+++ b/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
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index 8d73fc8bf59..b472a1ae81c 100644
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/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
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 343834f0a20..13b7e3588ce 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/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
--
GitLab