Re: [HACKERS] libpq and psql not on same page about SIGPIPE - Mailing list pgsql-patches

Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > Yes, that is the logic in my patch, except that I don't check errno, I
> > just call sigpending().
>
> No, that's wrong: if there is a pending SIGPIPE that belongs to the
> outer app, you'd clear it.

True, but I documented that in the patch.

> > There are a few writes and it seemed impossible
> > to check them all.
>
> Hmm?  There is only one place this needs to be done, namely
> pqsecure_write.

Look also in fe-print.c.  Looks like a lot of popen writes in there.
I can do it but it will be harder.

> BTW, have you posted the patch yet or are you still working on it?
> The mail server seems a bit flaky today ...

OK, patch attached.  I already sent it but who knows what happened to
it.

--
  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: configure
===================================================================
RCS file: /cvsroot/pgsql/configure,v
retrieving revision 1.409
diff -c -c -r1.409 configure
*** configure    30 Nov 2004 06:13:03 -0000    1.409
--- configure    1 Dec 2004 22:53:58 -0000
***************
*** 17431,17436 ****
--- 17431,17448 ----
  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
Index: configure.in
===================================================================
RCS file: /cvsroot/pgsql/configure.in,v
retrieving revision 1.387
diff -c -c -r1.387 configure.in
*** configure.in    30 Nov 2004 06:13:04 -0000    1.387
--- configure.in    1 Dec 2004 22:54:11 -0000
***************
*** 1174,1179 ****
--- 1174,1186 ----


  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
Index: doc/src/sgml/libpq.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/libpq.sgml,v
retrieving revision 1.169
diff -c -c -r1.169 libpq.sgml
*** doc/src/sgml/libpq.sgml    27 Nov 2004 21:56:04 -0000    1.169
--- doc/src/sgml/libpq.sgml    1 Dec 2004 22:54:45 -0000
***************
*** 3955,3975 ****
  </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>
--- 3955,3964 ----
  </para>

  <para>
! <application>libpq</application> blocks and discards <literal>SIGPIPE</>
! signals generated internally by <function>send()</> calls to the backend
! process. Therefore, applications should not expect blocked <literal>SIGPIPE</>
! signals to remain across <application>libpq</application> function calls.
  </para>

  <para>
Index: doc/src/sgml/ref/copy.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/ref/copy.sgml,v
retrieving revision 1.60
diff -c -c -r1.60 copy.sgml
*** doc/src/sgml/ref/copy.sgml    27 Nov 2004 21:56:05 -0000    1.60
--- doc/src/sgml/ref/copy.sgml    1 Dec 2004 22:54:57 -0000
***************
*** 3,8 ****
--- 3,9 ----
  PostgreSQL documentation
  -->

+
  <refentry id="SQL-COPY">
   <refmeta>
    <refentrytitle id="sql-copy-title">COPY</refentrytitle>
Index: src/interfaces/libpq/fe-connect.c
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.289
diff -c -c -r1.289 fe-connect.c
*** src/interfaces/libpq/fe-connect.c    30 Oct 2004 23:11:26 -0000    1.289
--- src/interfaces/libpq/fe-connect.c    1 Dec 2004 22:55:18 -0000
***************
*** 865,879 ****
      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;

--- 865,870 ----
Index: src/interfaces/libpq/fe-print.c
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-print.c,v
retrieving revision 1.55
diff -c -c -r1.55 fe-print.c
*** src/interfaces/libpq/fe-print.c    9 Nov 2004 15:57:57 -0000    1.55
--- src/interfaces/libpq/fe-print.c    1 Dec 2004 22:55:25 -0000
***************
*** 91,97 ****
          int            total_line_length = 0;
          int            usePipe = 0;
          char       *pagerenv;
!
  #if !defined(ENABLE_THREAD_SAFETY) && !defined(WIN32)
          pqsigfunc    oldsigpipehandler = NULL;
  #endif
--- 91,100 ----
          int            total_line_length = 0;
          int            usePipe = 0;
          char       *pagerenv;
! #ifdef ENABLE_THREAD_SAFETY
!         sigset_t    osigset;
!         bool        sigpipe_masked = false;
! #endif
  #if !defined(ENABLE_THREAD_SAFETY) && !defined(WIN32)
          pqsigfunc    oldsigpipehandler = NULL;
  #endif
***************
*** 189,195 ****
                  {
                      usePipe = 1;
  #ifdef ENABLE_THREAD_SAFETY
!                     pthread_setspecific(pq_thread_in_send, "t");
  #else
  #ifndef WIN32
                      oldsigpipehandler = pqsignal(SIGPIPE, SIG_IGN);
--- 192,199 ----
                  {
                      usePipe = 1;
  #ifdef ENABLE_THREAD_SAFETY
!                     pq_block_sigpipe(&osigset);
!                     sigpipe_masked = true;
  #else
  #ifndef WIN32
                      oldsigpipehandler = pqsignal(SIGPIPE, SIG_IGN);
***************
*** 311,317 ****
              pclose(fout);
  #endif
  #ifdef ENABLE_THREAD_SAFETY
!             pthread_setspecific(pq_thread_in_send, "f");
  #else
  #ifndef WIN32
              pqsignal(SIGPIPE, oldsigpipehandler);
--- 315,322 ----
              pclose(fout);
  #endif
  #ifdef ENABLE_THREAD_SAFETY
!             if (sigpipe_masked)
!                 pq_reset_sigpipe(&osigset);
  #else
  #ifndef WIN32
              pqsignal(SIGPIPE, oldsigpipehandler);
Index: src/interfaces/libpq/fe-secure.c
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-secure.c,v
retrieving revision 1.57
diff -c -c -r1.57 fe-secure.c
*** src/interfaces/libpq/fe-secure.c    20 Nov 2004 00:35:13 -0000    1.57
--- src/interfaces/libpq/fe-secure.c    1 Dec 2004 22:55:31 -0000
***************
*** 152,163 ****
  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                        */
  /* ------------------------------------------------------------ */
--- 152,157 ----
***************
*** 379,387 ****
  pqsecure_write(PGconn *conn, const void *ptr, size_t len)
  {
      ssize_t        n;
!
  #ifdef ENABLE_THREAD_SAFETY
!     pthread_setspecific(pq_thread_in_send, "t");
  #else
  #ifndef WIN32
      pqsigfunc    oldsighandler = pqsignal(SIGPIPE, SIG_IGN);
--- 373,383 ----
  pqsecure_write(PGconn *conn, const void *ptr, size_t len)
  {
      ssize_t        n;
!
  #ifdef ENABLE_THREAD_SAFETY
!     sigset_t    osigmask;
!
!     pq_block_sigpipe(&osigmask);
  #else
  #ifndef WIN32
      pqsigfunc    oldsighandler = pqsignal(SIGPIPE, SIG_IGN);
***************
*** 454,460 ****
          n = send(conn->sock, ptr, len, 0);

  #ifdef ENABLE_THREAD_SAFETY
!     pthread_setspecific(pq_thread_in_send, "f");
  #else
  #ifndef WIN32
      pqsignal(SIGPIPE, oldsighandler);
--- 450,456 ----
          n = send(conn->sock, ptr, len, 0);

  #ifdef ENABLE_THREAD_SAFETY
!     pq_reset_sigpipe(&osigmask);
  #else
  #ifndef WIN32
      pqsignal(SIGPIPE, oldsighandler);
***************
*** 1216,1280 ****
  }
  #endif   /* USE_SSL */

-
  #ifdef ENABLE_THREAD_SAFETY
- #ifndef WIN32
  /*
!  *    Check SIGPIPE handler and perhaps install our own.
   */
! 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)
  {
!     /*
!      * 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())
      {
!         if (pq_pipe_handler == SIG_DFL) /* not set by application */
!             exit(128 + SIGPIPE);    /* typical return value for SIG_DFL */
!         else
!             (*pq_pipe_handler) (signo); /* call original handler */
      }
- }
- #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.
-  */
- pqbool
- PQinSend(void)
- {
- #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

!     /*
!      * 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
  }
--- 1212,1264 ----
  }
  #endif   /* USE_SSL */

  #ifdef ENABLE_THREAD_SAFETY
  /*
!  *    Block SIGPIPE for this thread.  This prevents send()/write() from exiting
!  *    the application.
   */
! int
! pq_block_sigpipe(sigset_t *osigset)
  {
!     sigset_t sigpipe_sigset;
!
!     sigemptyset(&sigpipe_sigset);
!     sigaddset(&sigpipe_sigset, SIGPIPE);
!
!     /* Block SIGPIPE and save previous mask for later reset */
!     return pthread_sigmask(SIG_BLOCK, &sigpipe_sigset, osigset);
! }
!
! /*
!  *    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.
   */
! int
! pq_reset_sigpipe(sigset_t *osigset)
  {
!     int    signo;
!     sigset_t sigset;
!
!     /* Is SIGPIPE pending? */
!     if (sigpending(&sigset) != 0)
!         return -1;
!
!     if (sigismember(&sigset, SIGPIPE))
      {
!         sigset_t sigpipe_sigset;
!
!         sigemptyset(&sigpipe_sigset);
!         sigaddset(&sigpipe_sigset, SIGPIPE);
!
!         /* Discard pending and blocked SIGPIPE */
!         sigwait(&sigpipe_sigset, &signo);
!         if (signo != SIGPIPE)
!             return -1;
      }

!     /* Restore saved block mask */
!     return pthread_sigmask(SIG_SETMASK, osigset, NULL);
  }
+ #endif
Index: src/interfaces/libpq/libpq-fe.h
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/libpq-fe.h,v
retrieving revision 1.113
diff -c -c -r1.113 libpq-fe.h
*** src/interfaces/libpq/libpq-fe.h    30 Oct 2004 23:11:27 -0000    1.113
--- src/interfaces/libpq/libpq-fe.h    1 Dec 2004 22:55:33 -0000
***************
*** 497,508 ****

  /* === 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
--- 497,502 ----
Index: src/interfaces/libpq/libpq-int.h
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/libpq-int.h,v
retrieving revision 1.96
diff -c -c -r1.96 libpq-int.h
*** src/interfaces/libpq/libpq-int.h    30 Oct 2004 23:11:27 -0000    1.96
--- src/interfaces/libpq/libpq-int.h    1 Dec 2004 22:55:35 -0000
***************
*** 31,36 ****
--- 31,37 ----

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

  #ifdef WIN32_CLIENT_ONLY
***************
*** 475,489 ****
  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

  /*
   * this is so that we can check if a connection is non-blocking internally
   * without the overhead of a function call
--- 476,490 ----
  extern ssize_t pqsecure_read(PGconn *, void *ptr, size_t len);
  extern ssize_t pqsecure_write(PGconn *, const void *ptr, size_t len);

  #ifdef USE_SSL
  extern bool pq_initssllib;
  #endif

+ #ifdef ENABLE_THREAD_SAFETY
+ int pq_block_sigpipe(sigset_t *osigset);
+ int pq_reset_sigpipe(sigset_t *osigset);
+ #endif
+
  /*
   * this is so that we can check if a connection is non-blocking internally
   * without the overhead of a function call