Re: SIGPIPE handling

Bruce Momjian wrote:

>Better.  However, I am confused over when we do sigaction.  I thought we
>were going to do it only if they had a signal handler defined, meaning
>
>    if (pipehandler != SIG_DFL &&
>        pipehandler != SIG_IGN &&
>        pipehandler != SIG_ERR)
>        conn->do_sigaction = true;
>    else
>        conn->do_sigaction = false;
>
>By doing this, we don't do sigaction in the default case where no
>handler was defined.
>
No. If no handler was definied, then libpq must define a handler.
Without a handler, a network disconnect would result in a SIGPIE that
kills the app.

>  I thought we would just set the entire application
>to SIGPIPE <= SIG_IGN.  This gives us good performance in all cases
>except when a signal handler is defined.
>
I don't want to change the whole app - perhaps someone expects that
sigpipe works? Perhaps psql for the console input, or something similar?

>  Is running the rest of the
>application with SIGPIPE <= SIG_IGN a problem?
>
>
I think that depends on the application, and libpq shouldn't mandate
that SIGPIPE must be SIG_IGN. Right now libpq tries to catch sigpipe
signals by manually installing/restoring a signal handler around send()
calls. This doesn't work for multithreaded apps, because the signal
handlers are per-process, not per-thread.

Thus for multithreaded apps, the libpq user is responsible for handling
sigpipe. The API change should be a big problem - the current system
doesn't work, and there shouldn't be many multithreaded apps.

But how should libpq notice that the caller handles sigpipe signals?
a) autodetection - if the sigpipe handler is not the default, then the
caller knows what he's doing.
b) a new PGsetsignalhandler() function.
c) an additional flag passed to PGconnectdb.

Tom preferred a). One problem is that the autodetection is not perfect:
an app could block the signal with sigprocmask, or it could install a
handler that doesn't expect sigpipe signals from within libpq.
I would prefer b), because it guarantees that the patch has no effect on
existing apps.
c) is bad, Tom explained that the connect string is often directly
specified by the user.

--
    Manfred