Re: SIGPIPE handling - Mailing list pgsql-patches

Here is my solution to ignoring SIGPIPE in libpq's send() for threaded
apps.

It defines a custom SIGPIPE handler if one is not already defined by the
application, and uses a thread-local variable that is checked in the
SIGPIPE handler to know if the SIGPIPE was caused by a libpq send()
call.

The documentation is at the top of the patch.  Changed from the
description below is that applications can define their own SIGPIPE
handler after establishing the first database connection.  However,
custom SIGPIPE handlers must check PQinSend() to determine if the signal
should be ignored.

---------------------------------------------------------------------------

pgman wrote:
>
> Attached is my idea for implementing safe SIGPIPE in threaded apps.  The
> code has the same libpq behavior if not compiled using
> --enable-thread-safety.
>
> If compiled with that option, an app wanting to define its own SIGPIPE
> handler has to do so before connecting to a database.  On first
> connection, the code checks to see if there is a SIGPIPE handler, and if
> not, installs its own, and creates a thread-local variable.  Then, on
> each send(), it sets, calls send(), then clears the thread-local
> variable.  The SIGPIPE handler checks the thread-local variable and
> either ignores or exits depending on whether it was in send().
>
> Right now the thread-local variable is static to the file, but we could
> export it as a boolean so custom SIGPIPE handlers could check it and
> take action or ignore the signal just like our code.  Not sure if that
> is a good idea or not.  In fact, even cleaner, we could create a
> function that allows users to define their own SIGPIPE handler and it
> would be called only when not called by libpq send(), and it would work
> safely for threaded apps.
>
> I think the big problem with my approach is that it requires special
> custom SIGPIPE handler code even if the app isn't multi-threaded but
> libpq is compiled as multi-threaded.
>
> Another idea is to create PQsigpipefromsend() that returns true/false
> depending on whether the SIGPIPE was from libpq's send().  It could be a
> global variable set/cleared in non-threaded libpq and a thread-local
> variable in threaded libpq.  It would allow the same API/behavior for
> both libpq versions and all custom SIGPIPE handlers using libpq would
> have to check it.
>
> The one good thing about the patch is that it ignores send() SIGPIPE,
> and gives default SIG_DFL behavior for libpq apps with no special app
> coding, with the downside of requiring extra cost for custom SIGPIPE
> handlers.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
Index: doc/src/sgml/libpq.sgml
===================================================================
RCS file: /cvsroot/pgsql-server/doc/src/sgml/libpq.sgml,v
retrieving revision 1.144
diff -c -c -r1.144 libpq.sgml
*** doc/src/sgml/libpq.sgml    13 Dec 2003 23:59:06 -0000    1.144
--- doc/src/sgml/libpq.sgml    7 Jan 2004 21:42:56 -0000
***************
*** 3587,3593 ****
  One restriction is that no two threads attempt to manipulate the same
  <structname>PGconn</> object at the same time. In particular, you cannot
  issue concurrent commands from different threads through the same
! connection object. (If you need to run concurrent commands, start up
  multiple connections.)
  </para>

--- 3587,3593 ----
  One restriction is that no two threads attempt to manipulate the same
  <structname>PGconn</> object at the same time. In particular, you cannot
  issue concurrent commands from different threads through the same
! connection object. (If you need to run concurrent commands, use
  multiple connections.)
  </para>

***************
*** 3611,3616 ****
--- 3611,3635 ----
  not thread-safe.<indexterm><primary>crypt</><secondary>thread
  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 if no custom <literal>SIGPIPE</>
+ handler has been installed previously. This handler uses thread-local
+ storage to determine if a <literal>SIGPIPE</> signal has been generated
+ by an internal <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>
  </sect1>

Index: src/backend/nodes/read.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/nodes/read.c,v
retrieving revision 1.36
diff -c -c -r1.36 read.c
*** src/backend/nodes/read.c    29 Nov 2003 19:51:49 -0000    1.36
--- src/backend/nodes/read.c    7 Jan 2004 21:42:56 -0000
***************
*** 22,27 ****
--- 22,28 ----
  #include <ctype.h>
  #include <errno.h>

+ #include "nodes/value.h"
  #include "nodes/pg_list.h"
  #include "nodes/readfuncs.h"

Index: src/interfaces/libpq/fe-connect.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.266
diff -c -c -r1.266 fe-connect.c
*** src/interfaces/libpq/fe-connect.c    7 Jan 2004 18:56:29 -0000    1.266
--- src/interfaces/libpq/fe-connect.c    7 Jan 2004 21:43:01 -0000
***************
*** 43,48 ****
--- 43,52 ----
  #include <arpa/inet.h>
  #endif

+ #ifdef ENABLE_THREAD_SAFETY
+ #include <pthread.h>
+ #endif
+
  #include "libpq/ip.h"
  #include "mb/pg_wchar.h"

***************
*** 66,72 ****
  #define DefaultSSLMode    "disable"
  #endif

-
  /* ----------
   * Definition of the conninfo parameters and their fallback resources.
   *
--- 70,75 ----
***************
*** 198,203 ****
--- 201,207 ----
  static char *PasswordFromFile(char *hostname, char *port, char *dbname,
                   char *username);

+
  /*
   *        Connecting to a Database
   *
***************
*** 881,886 ****
--- 885,896 ----
      struct addrinfo hint;
      const char *node = NULL;
      int            ret;
+ #ifdef ENABLE_THREAD_SAFETY
+     static pthread_once_t check_sigpipe_once = PTHREAD_ONCE_INIT;
+
+     /* Check only on first connection request */
+     pthread_once(&check_sigpipe_once, check_sigpipe_handler);
+ #endif

      if (!conn)
          return 0;
***************
*** 3158,3160 ****
--- 3168,3171 ----

  #undef LINELEN
  }
+
Index: src/interfaces/libpq/fe-print.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/interfaces/libpq/fe-print.c,v
retrieving revision 1.49
diff -c -c -r1.49 fe-print.c
*** src/interfaces/libpq/fe-print.c    29 Nov 2003 19:52:12 -0000    1.49
--- src/interfaces/libpq/fe-print.c    7 Jan 2004 21:43:01 -0000
***************
*** 90,97 ****
          int            fs_len = strlen(po->fieldSep);
          int            total_line_length = 0;
          int            usePipe = 0;
-         pqsigfunc    oldsigpipehandler = NULL;
          char       *pagerenv;

  #ifdef TIOCGWINSZ
          struct winsize screen_size;
--- 90,99 ----
          int            fs_len = strlen(po->fieldSep);
          int            total_line_length = 0;
          int            usePipe = 0;
          char       *pagerenv;
+ #if !defined(ENABLE_THREAD_SAFETY) && !defined(WIN32)
+         pqsigfunc    oldsigpipehandler = NULL;
+ #endif

  #ifdef TIOCGWINSZ
          struct winsize screen_size;
***************
*** 189,197 ****
--- 191,203 ----
                  if (fout)
                  {
                      usePipe = 1;
+ #ifdef ENABLE_THREAD_SAFETY
+                     pthread_setspecific(thread_in_send, "t");
+ #else
  #ifndef WIN32
                      oldsigpipehandler = pqsignal(SIGPIPE, SIG_IGN);
  #endif
+ #endif
                  }
                  else
                      fout = stdout;
***************
*** 306,312 ****
--- 312,324 ----
              _pclose(fout);
  #else
              pclose(fout);
+ #endif
+ #ifdef ENABLE_THREAD_SAFETY
+             pthread_setspecific(thread_in_send, "f");
+ #else
+ #ifndef WIN32
              pqsignal(SIGPIPE, oldsigpipehandler);
+ #endif
  #endif
          }
          if (po->html3 && !po->expanded)
Index: src/interfaces/libpq/fe-secure.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/interfaces/libpq/fe-secure.c,v
retrieving revision 1.34
diff -c -c -r1.34 fe-secure.c
*** src/interfaces/libpq/fe-secure.c    18 Dec 2003 22:49:26 -0000    1.34
--- src/interfaces/libpq/fe-secure.c    7 Jan 2004 21:43:02 -0000
***************
*** 106,111 ****
--- 106,115 ----
  #include <arpa/inet.h>
  #endif

+ #ifdef ENABLE_THREAD_SAFETY
+ #include <pthread.h>
+ #endif
+
  #ifndef HAVE_STRDUP
  #include "strdup.h"
  #endif
***************
*** 142,147 ****
--- 146,156 ----
  static SSL_CTX *SSL_context = NULL;
  #endif

+ #ifdef ENABLE_THREAD_SAFETY
+ static void sigpipe_handler_ignore_send(int signo);
+ pthread_key_t thread_in_send;
+ #endif
+
  /* ------------------------------------------------------------ */
  /*                         Hardcoded values                        */
  /* ------------------------------------------------------------ */
***************
*** 347,355 ****
--- 356,368 ----
  {
      ssize_t        n;

+ #ifdef ENABLE_THREAD_SAFETY
+     pthread_setspecific(thread_in_send, "t");
+ #else
  #ifndef WIN32
      pqsigfunc    oldsighandler = pqsignal(SIGPIPE, SIG_IGN);
  #endif
+ #endif

  #ifdef USE_SSL
      if (conn->ssl)
***************
*** 407,415 ****
--- 420,432 ----
  #endif
          n = send(conn->sock, ptr, len, 0);

+ #ifdef ENABLE_THREAD_SAFETY
+     pthread_setspecific(thread_in_send, "f");
+ #else
  #ifndef WIN32
      pqsignal(SIGPIPE, oldsighandler);
  #endif
+ #endif

      return n;
  }
***************
*** 1048,1050 ****
--- 1065,1123 ----
  }

  #endif   /* USE_SSL */
+
+
+ #ifdef ENABLE_THREAD_SAFETY
+ /*
+  *    Check SIGPIPE handler and perhaps install our own.
+  */
+ void
+ check_sigpipe_handler(void)
+ {
+     pqsigfunc pipehandler;
+
+     /*
+      *    If the app hasn't set a SIGPIPE handler, define our own
+      *    that ignores SIGPIPE on libpq send() and does SIG_DFL
+      *    for other SIGPIPE cases.
+      */
+     pipehandler = pqsignalinquire(SIGPIPE);
+     if (pipehandler == SIG_DFL)    /* not set by application */
+     {
+         /*
+          *    Create key first because the signal handler might be called
+          *    right after being installed.
+          */
+         pthread_key_create(&thread_in_send, NULL);
+         pqsignal(SIGPIPE, sigpipe_handler_ignore_send);
+     }
+ }
+
+ /*
+  *    Threaded SIGPIPE signal handler
+  */
+ void
+ sigpipe_handler_ignore_send(int signo)
+ {
+     /* If we have gotten a SIGPIPE outside send(), exit */
+     if (!PQinSend())
+         exit(128 + SIGPIPE);    /* typical return value for SIG_DFL */
+ }
+ #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.
+  */
+ pqbool
+ PQinSend(void)
+ {
+ #ifdef ENABLE_THREAD_SAFETY
+     return (pthread_getspecific(thread_in_send) /* has it been set? */ &&
+             *(char *)pthread_getspecific(thread_in_send) == 't') ? true : false;
+ #else
+     return false;    /* No threading, so we can't be in send() */
+ #endif
+ }
Index: src/interfaces/libpq/libpq-fe.h
===================================================================
RCS file: /cvsroot/pgsql-server/src/interfaces/libpq/libpq-fe.h,v
retrieving revision 1.101
diff -c -c -r1.101 libpq-fe.h
*** src/interfaces/libpq/libpq-fe.h    29 Nov 2003 22:41:28 -0000    1.101
--- src/interfaces/libpq/libpq-fe.h    7 Jan 2004 21:43:03 -0000
***************
*** 450,455 ****
--- 450,463 ----
  /* Get encoding id from environment variable PGCLIENTENCODING */
  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().
+  */
+ pqbool PQinSend(void);
+
  #ifdef __cplusplus
  }
  #endif
Index: src/interfaces/libpq/libpq-int.h
===================================================================
RCS file: /cvsroot/pgsql-server/src/interfaces/libpq/libpq-int.h,v
retrieving revision 1.83
diff -c -c -r1.83 libpq-int.h
*** src/interfaces/libpq/libpq-int.h    29 Nov 2003 22:41:28 -0000    1.83
--- src/interfaces/libpq/libpq-int.h    7 Jan 2004 21:43:03 -0000
***************
*** 29,34 ****
--- 29,37 ----
  #include <sys/time.h>
  #endif

+ #ifdef ENABLE_THREAD_SAFETY
+ #include <pthread.h>
+ #endif

  #if defined(WIN32) && (!defined(ssize_t))
  typedef int ssize_t;            /* ssize_t doesn't exist in VC (at least
***************
*** 442,447 ****
--- 445,454 ----
  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 check_sigpipe_handler(void);
+ extern pthread_key_t thread_in_send;
+ #endif

  /*
   * this is so that we can check if a connection is non-blocking internally
Index: src/interfaces/libpq/pqsignal.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/interfaces/libpq/pqsignal.c,v
retrieving revision 1.18
diff -c -c -r1.18 pqsignal.c
*** src/interfaces/libpq/pqsignal.c    29 Nov 2003 19:52:12 -0000    1.18
--- src/interfaces/libpq/pqsignal.c    7 Jan 2004 21:43:03 -0000
***************
*** 40,42 ****
--- 40,66 ----
      return oact.sa_handler;
  #endif   /* !HAVE_POSIX_SIGNALS */
  }
+
+ pqsigfunc
+ pqsignalinquire(int signo)
+ {
+ #if !defined(HAVE_POSIX_SIGNALS)
+     pqsigfunc old;
+
+     /*
+      *    We could lose a signal during this test.
+      *    In a multi-threaded application, this might
+      *    be a problem.  Do any non-threaded platforms
+      *    lack sigaction()?
+      */
+      old = signal(signo, SIG_IGN);
+     signal(signo, old);
+     return old;
+ #else
+     struct sigaction oact;
+
+     if (sigaction(signo, NULL, &oact) < 0)
+        return SIG_ERR;
+     return oact.sa_handler;
+ #endif   /* !HAVE_POSIX_SIGNALS */
+ }
Index: src/interfaces/libpq/pqsignal.h
===================================================================
RCS file: /cvsroot/pgsql-server/src/interfaces/libpq/pqsignal.h,v
retrieving revision 1.16
diff -c -c -r1.16 pqsignal.h
*** src/interfaces/libpq/pqsignal.h    29 Nov 2003 22:41:28 -0000    1.16
--- src/interfaces/libpq/pqsignal.h    7 Jan 2004 21:43:03 -0000
***************
*** 24,27 ****
--- 24,29 ----

  extern pqsigfunc pqsignal(int signo, pqsigfunc func);

+ extern pqsigfunc pqsignalinquire(int signo);
+
  #endif   /* PQSIGNAL_H */