Re: SIGPIPE handling - Mailing list pgsql-patches

Manfred Spraul wrote:
> Bruce Momjian wrote:
>
> >OK, I know you had a flag for pgbench, and that doesn't use threads.
> >What speedup do you see there?
> >
> >
> Tiny. I added the flag to check that my implementation works, not as a
> benchmark tool.
>
> >I would not expect a library to require me to do something in my code to
> >be thread-safe --- either it is or it isn't.
> >
> The library is thread-safe. Just the SIGPIPE handling differs:
> - single thread: handled by libpq.
> - multi thread: caller must handle SIGPIPE for libpq.
> Rationale: posix is broken. Per-thread signal handling is too ugly to
> think about.

I can accept that we require special code in the app to be thread-safe
_if_ they are installing their own SIGPIPE handler, but I don't think it
is fair to require them to set SIGPIPE ==> SIG_IGN to be thread-safe.

> >Again, let's get it working perfect if they say they are going to use
> >threads with libpq.  Does it work OK if the app doesn't use threading?
> >
> >
> No. pthread_sigmask is part of libpthread - libpq would have to link
> unconditionally against libpthread. Or use __attribute__((weak,
> alias())), but that would only work with gcc.

libpq already links against any thread libraries if you configure
--enable-thread-safety.  If you don't, we don't have to be thread-safe.
My question was whether a non-threaded app handles pthread_sigmask in a
normal way or does it only work when you are running in a thread,
pthread_create()?

> >Does sigpending/sigwait work efficiently for threads?  Another idea is
> >to go with a thread-local storage boolean for each thread, and check
> >that in a signal handler we install.
> >
> I think installing a signal handler is not an option - libpq is a
> library, the signal handler is global.

OK.  My suggestion was to add a libpq C function to register a SIGPIPE
handler.  That way, if they don't call it, we can install our own and
handle it via SIG_IGN (if in send()), or SIG_DFL (if not in send()).

If they install their own, they have to handle ignoring SIGPIPE from
send().  They can use our code as an example.

You say you don't want to install a SIGPIPE signal handler, but we are
requiring code to make SIGPIPE => SIG_IGN to be thread-safe.  That seems
like a pretty strange burden that most threaded apps will not figure out
without a lot of digging.  And if you try to install a custom SIGPIPE
handler in a threaded app, libpq will not even be thread-safe because
their signal handler will be called from send() and they have no way to
determine when to ignore it (coming from send()).  Whatever the
solution, I would like to have something that requires a minimal change
in application code, and works reliably in a threaded app.

On the one hand, you are saying libpq shouldn't install a signal
handler, and in another you are saying you have to set SIGPIPE to
SIG_IGN for the library to be thread-safe.

> >  Seems synchronous signals like
> >SIGPIPE are delivered to the thread that invoked them, and we can check
> >thread-local storage to see if we were in a send() loop at the time of
> >signal delivery.
> >
> >
> IMHO way to fragile.

Why?  We have to do something reasonable?  I don't like requiring
SIGPIPE => SIG_IGN to be thread-safe.

Let's look at our four use cases:

    non-threaded app, no SIGPIPE handler - works fine now
    non-threaded app, custom SIGPIPE handler - works fine now
    threaded app, no SIGPIPE handler - doesn't work
    threaded app, custom SIGPIPE handler - doesn't work

I assume we want to get those last two working without breaking the
earlier ones.  I suppose the main argument to _not_ installing our own
SIGPIPE handler is that it would require special work for non-threaded
apps that want to install their own SIGPIPE handler --- they would have
to install the handler _before_ they open a libpq connection, and they
would have to deal with checking the thread-specific send() boolean in
their signal handler to determine if they should ignore the signal.
That does sound like a mess, and is required in non-threaded apps, which
right now work fine without special checking in the custom SIGPIPE
handler.

I thought someone said that an app shouldn't ignore SIGPIPE everywhere?
What happens if an app does that?  I assume using the app in a unix pipe
case would cause the process not to die when the input pipe is closed or
the output pipe closed.  That seems strange.

I was thinking of using pthread_setspecific() and pthread_getspecific()
around the send() call, and have the SIGPIPE signal handler ignore the
signal if it came from the send() block --- you set/clear the value
before/after send().

Should I try to code up something everyone can look at?

--
  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