Re: SIGPIPE handling

From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: Manfred Spraul <manfred(at)colorfullife(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL-patches <pgsql-patches(at)postgresql(dot)org>
Subject: Re: SIGPIPE handling
Date: 2003-11-18 23:19:15
Message-ID: 200311182319.hAINJFt03892@candle.pha.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-patches


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.

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

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

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

Attachment Content-Type Size
unknown_filename text/plain 6.4 KB

In response to

Browse pgsql-patches by date

  From Date Subject
Next Message Thomas Baden 2003-11-19 03:17:44 7.4 shared memory error on 64-bit SPARC/Solaris 5.8
Previous Message Sergey N. Yatskevich 2003-11-18 23:07:43 Bug in byteaout code in all PostgreSQL versions