Re: SIGPIPE handling
Here is my solution to ignoring SIGPIPE in libpq's send() for threaded
apps.
It defines a custom SIGPIPE handler if one is not already defined by the
application, and uses a thread-local variable that is checked in the
SIGPIPE handler to know if the SIGPIPE was caused by a libpq send()
call.
The documentation is at the top of the patch. Changed from the
description below is that applications can define their own SIGPIPE
handler after establishing the first database connection. However,
custom SIGPIPE handlers must check PQinSend() to determine if the signal
should be ignored.
---------------------------------------------------------------------------
pgman wrote:
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.
--
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 19073Attachments:
/pgpatches/sigpipetext/plainDownload+161-4
Import Notes
Reply to msg id not found:
One issue I had is in the following function. How can I easily find the
current signal value without causing possible signal loss during
testing, or possible abort if signals were previously ignored. I could
use sigblock, and I think that would exist on a system that doesn't have
sigaction, but is it worth the portability issue? Does any platform
have threads and not sigaction?
---------------------------------------------------------------------------
+ + pqsigfunc + pqsignalinquire(int signo) + { + #if !defined(HAVE_POSIX_SIGNALS) + pqsigfunc old; + + /* + * We could lose a signal during this test. + * In a multi-threaded application, this might + * be a problem. Do any non-threaded platforms + * lack sigaction()? + */ + old = signal(signo, SIG_IGN); + signal(signo, old); + return old; + #else + struct sigaction oact; + + if (sigaction(signo, NULL, &oact) < 0) + return SIG_ERR; + return oact.sa_handler; + #endif /* !HAVE_POSIX_SIGNALS */ + }
--
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
Bruce Momjian wrote:
+ /* + * We could lose a signal during this test. + * In a multi-threaded application, this might + * be a problem. Do any non-threaded platforms
Threaded or non-threaded?
+ * lack sigaction()?
+ */
Additionally, the problem is not restricted to multithreaded apps:
signal(,SIG_IGN) clears all pending signals.
--
Manfred
Manfred Spraul wrote:
Bruce Momjian wrote:
+ /* + * We could lose a signal during this test. + * In a multi-threaded application, this might + * be a problem. Do any non-threaded platformsThreaded or non-threaded?
OK, yea, I will use threaded.
+ * lack sigaction()?
+ */Additionally, the problem is not restricted to multithreaded apps:
signal(,SIG_IGN) clears all pending signals.
Oh, yuck. Would SIG_DFL be better here? I am thinking of adding
sigblock into that code on the assumption that if they have signal(),
they have sigblock(). Should we disable threaded builds unless they
have sigaction()?
I suppose the sigblock() would take care of the pending signal problem
too.
--
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 19073Manfred Spraul wrote:
Bruce Momjian wrote:
+ /* + * We could lose a signal during this test. + * In a multi-threaded application, this might + * be a problem. Do any non-threaded platformsThreaded or non-threaded?
+ * lack sigaction()?
+ */Additionally, the problem is not restricted to multithreaded apps:
signal(,SIG_IGN) clears all pending signals.
OK, new function using sigblock():
pqsigfunc
pqsignalinquire(int signo)
{
#if !defined(HAVE_POSIX_SIGNALS)
pqsigfunc old_sigfunc;
int old_sigmask;
/* Prevent signal handler calls during test */
old_sigmask = sigblock(sigmask(signo));
old_sigfunc = signal(signo, SIG_DFL);
signal(signo, old_sigfunc);
sigblock(old_sigmask);
return old_sigfunc;
#else
struct sigaction oact;
if (sigaction(signo, NULL, &oact) < 0)
return SIG_ERR;
return oact.sa_handler;
#endif /* !HAVE_POSIX_SIGNALS */
}
--
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