I think this is the patch I like.  It does the auto-detect handling as I
hoped.  I will just do the doc updates to mention it.

My only issue is that this is per-connection, while I think you have to
create a global variable that defaults to false, and on first connect,
check, and not after.  Based on the code below, a second connection
would  have the SIGPIPE signal set to SIG_IGN, not SIG_DEF, and you
would be back to setting SIG_IGN around each send, even though it was
already set.

Are others OK with this too?

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

Manfred Spraul wrote:
> pqsecure_write tries to catch SIGPIPE signals generated by network 
> disconnects by setting the signal handler to SIG_IGN. The current 
> approach causes several problems:
> - it always sets SA_RESTART when it restores the old handler.
> - it's not reliable for multi threaded apps, because another thread 
> could change the signal handler inbetween.
> - it's slow, because after setting a signal handler to SIG_IGN the 
> kernel must enumerate all threads and clear all pending signals (at 
> least FreeBSD-5.1 and linux-2.6 do that. Earlier linux kernels don't - 
> their signal handling is known to be broken for multithreaded apps).
> 
> Initially I proposed a new option for PQconnectdb, but Tom didn't like 
> that. The attached patch autodetects if it should set the signal 
> handler, Tom proposed that. The code doesn't try to check if the signal 
> is "handled" by blocking it, because I haven't figured out how to check 
> that: sigprocmask is undefined for multithreaded apps and calling 
> pthread_sigmask would force every libpq user to link against libpthread.
> 
> --
>     Manfred

> ? src/interfaces/libpq/libpq.so.3.1
> Index: src/interfaces/libpq/fe-connect.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/fe-connect.c,v
> retrieving revision 1.263
> diff -c -r1.263 fe-connect.c
> *** src/interfaces/libpq/fe-connect.c 18 Oct 2003 05:02:06 -0000      1.263
> --- src/interfaces/libpq/fe-connect.c 2 Nov 2003 18:29:40 -0000
> ***************
> *** 41,46 ****
> --- 41,47 ----
>   #include <netinet/tcp.h>
>   #endif
>   #include <arpa/inet.h>
> + #include <signal.h>
>   #endif
>   
>   #include "libpq/ip.h"
> ***************
> *** 951,956 ****
> --- 952,983 ----
>       else if (conn->sslmode[0] == 'a')       /* "allow" */
>               conn->wait_ssl_try = true;
>   #endif
> +     /* 
> +      * Autodetect SIGPIPE signal handling:
> +      * The default action per Unix spec is kill current process and
> +      * that's not acceptable. If the current setting is not the default,
> +      * then assume that the caller knows what he's doing and leave the
> +      * signal handler unchanged. Otherwise set the signal handler to
> +      * SIG_IGN around each send() syscall. Unfortunately this is both
> +      * unreliable and slow for multithreaded apps.
> +      */
> +     conn->do_sigaction = true;
> + #if !defined(HAVE_POSIX_SIGNALS)
> +     {
> +             pqsigfunc old;
> +             old = signal(SIGPIPE, SIG_IGN);
> +             if (old != SIG_DFL)
> +                     conn->do_sigaction = false;
> +             signal(SIGPIPE, old);
> +     }
> + #else
> +     {
> +             struct sigaction oact;
> + 
> +             if (sigaction(SIGPIPE, NULL, &oact) == 0 && oact.sa_handler != SIG_DFL)
> +                     conn->do_sigaction = false;
> +     }
> + #endif   /* !HAVE_POSIX_SIGNALS */
>   
>       /*
>        * Set up to try to connect, with protocol 3.0 as the first attempt.
> Index: src/interfaces/libpq/fe-secure.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/fe-secure.c,v
> retrieving revision 1.32
> diff -c -r1.32 fe-secure.c
> *** src/interfaces/libpq/fe-secure.c  29 Sep 2003 16:38:04 -0000      1.32
> --- src/interfaces/libpq/fe-secure.c  2 Nov 2003 18:29:41 -0000
> ***************
> *** 348,354 ****
>       ssize_t         n;
>   
>   #ifndef WIN32
> !     pqsigfunc       oldsighandler = pqsignal(SIGPIPE, SIG_IGN);
>   #endif
>   
>   #ifdef USE_SSL
> --- 348,357 ----
>       ssize_t         n;
>   
>   #ifndef WIN32
> !     pqsigfunc       oldsighandler = NULL;
> ! 
> !     if (conn->do_sigaction)
> !             oldsighandler = pqsignal(SIGPIPE, SIG_IGN);
>   #endif
>   
>   #ifdef USE_SSL
> ***************
> *** 408,414 ****
>               n = send(conn->sock, ptr, len, 0);
>   
>   #ifndef WIN32
> !     pqsignal(SIGPIPE, oldsighandler);
>   #endif
>   
>       return n;
> --- 411,418 ----
>               n = send(conn->sock, ptr, len, 0);
>   
>   #ifndef WIN32
> !     if (conn->do_sigaction)
> !             pqsignal(SIGPIPE, oldsighandler);
>   #endif
>   
>       return n;
> Index: src/interfaces/libpq/libpq-int.h
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/libpq/libpq-int.h,v
> retrieving revision 1.82
> diff -c -r1.82 libpq-int.h
> *** src/interfaces/libpq/libpq-int.h  5 Sep 2003 02:08:36 -0000       1.82
> --- src/interfaces/libpq/libpq-int.h  2 Nov 2003 18:29:42 -0000
> ***************
> *** 329,334 ****
> --- 329,335 ----
>       char            peer_dn[256 + 1];               /* peer distinguished name */
>       char            peer_cn[SM_USER + 1];   /* peer common name */
>   #endif
> +     bool            do_sigaction;   /* set SIGPIPE to SIG_IGN around every send() 
> call */
>   
>       /* Buffer for current error message */
>       PQExpBufferData errorMessage;           /* expansible string */

> 
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Don't 'kill -9' the postmaster

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  [EMAIL PROTECTED]               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

---------------------------(end of broadcast)---------------------------
TIP 9: the planner will ignore your desire to choose an index scan if your
      joining column's datatypes do not match

Reply via email to