Re: [PATCHES] SIGPIPE handling

2004-01-08 Thread Bruce Momjian
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 platforms > >> > Threaded or non-threaded? > > >>+* lack

Re: [PATCHES] SIGPIPE handling

2004-01-08 Thread Bruce Momjian
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 platforms > >> > Threaded or non-threaded? OK, yea, I will use th

Re: [PATCHES] SIGPIPE handling

2004-01-07 Thread Manfred Spraul
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 p

Re: [PATCHES] SIGPIPE handling

2004-01-07 Thread Bruce Momjian
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

Re: [PATCHES] SIGPIPE handling

2004-01-07 Thread Bruce Momjian
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

Re: [PATCHES] SIGPIPE handling

2003-11-18 Thread Bruce Momjian
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

Re: [PATCHES] SIGPIPE handling

2003-11-17 Thread Bruce Momjian
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 requ

Re: [PATCHES] SIGPIPE handling

2003-11-17 Thread Manfred Spraul
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-s

Re: [PATCHES] SIGPIPE handling

2003-11-17 Thread Bruce Momjian
Manfred Spraul wrote: > Bruce Momjian wrote: > > >Here is my logic --- 99% of apps don't install a SIGPIPE signal handler, > >and 90% will not add a SIGPIPE/SIG_IGN call to their applications. I > >guess I am looking for something that would allow the performance > >benefit of not doing a pgsigna

Re: [PATCHES] SIGPIPE handling

2003-11-16 Thread Manfred Spraul
Bruce Momjian wrote: Here is my logic --- 99% of apps don't install a SIGPIPE signal handler, and 90% will not add a SIGPIPE/SIG_IGN call to their applications. I guess I am looking for something that would allow the performance benefit of not doing a pgsignal() call around very send() for the ma

Re: [PATCHES] SIGPIPE handling

2003-11-16 Thread Bruce Momjian
Manfred Spraul wrote: > Bruce Momjian wrote: > > >I thought it should be global too, basically testing on the first > >connection request. > > > What if two PQconnect calls happen at the same time? > I would really prefer the manual approach with a new PQsetsighandler > function - the autodetecti

Re: [PATCHES] SIGPIPE handling

2003-11-16 Thread Bruce Momjian
Tom Lane wrote: > Bruce Momjian <[EMAIL PROTECTED]> writes: > > Yes, I was afraid of that. Here's another idea. If the signal handler > > is SIG_DFL, we install our own signal handler for SIGPIPE, and set/clear a > > global variable before/after we send(). > > That would address the speed issue

Re: [PATCHES] SIGPIPE handling

2003-11-16 Thread Tom Lane
Manfred Spraul <[EMAIL PROTECTED]> writes: > + extern void PQsetsighandling(int internal_sigign); These sorts of things are commonly designed so that the set() operation incidentally returns the previous setting. I'm not sure if anyone would care, but it's only a couple more lines of code to make

Re: [PATCHES] SIGPIPE handling

2003-11-16 Thread Manfred Spraul
Bruce Momjian wrote: I thought it should be global too, basically testing on the first connection request. What if two PQconnect calls happen at the same time? I would really prefer the manual approach with a new PQsetsighandler function - the autodetection is fragile, it's trivial to find a spec

Re: [PATCHES] SIGPIPE handling

2003-11-16 Thread Bruce Momjian
Tom Lane wrote: > Manfred Spraul <[EMAIL PROTECTED]> writes: > > 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 a

Re: [PATCHES] SIGPIPE handling

2003-11-16 Thread Tom Lane
Bruce Momjian <[EMAIL PROTECTED]> writes: > Yes, I was afraid of that. Here's another idea. If the signal handler > is SIG_DFL, we install our own signal handler for SIGPIPE, and set/clear a > global variable before/after we send(). That would address the speed issue but not the multithread corr

Re: [PATCHES] SIGPIPE handling

2003-11-16 Thread Bruce Momjian
Tom Lane wrote: > Bruce Momjian <[EMAIL PROTECTED]> writes: > > Is running the rest of the > > application with SIGPIPE <= SIG_IGN a problem? > > That is NOT an acceptable thing for a library to do. Yes, I was afraid of that. Here's another idea. If the signal handler is SIG_DFL, we install our

Re: [PATCHES] SIGPIPE handling

2003-11-16 Thread Tom Lane
Kurt Roeckx <[EMAIL PROTECTED]> writes: > On Sun, Nov 16, 2003 at 06:28:06PM +0100, Kurt Roeckx wrote: >> Is there a reason we don't make use of the MSG_NOSIGNAL flag to >> send()? Or is the problem in case of SSL? > Oh, seems to be a Linux only thing? That and the SSL problem. I wouldn't objec

Re: [PATCHES] SIGPIPE handling

2003-11-16 Thread Tom Lane
Manfred Spraul <[EMAIL PROTECTED]> writes: > 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 PGc

Re: [PATCHES] SIGPIPE handling

2003-11-16 Thread Kurt Roeckx
On Sun, Nov 16, 2003 at 06:28:06PM +0100, Kurt Roeckx wrote: > On Sun, Nov 16, 2003 at 12:56:10PM +0100, Manfred Spraul wrote: > > Hi, > > > > attached is an update of my automatic sigaction patch: I've moved the > > actual sigaction calls into pqsignal.c and added a helper function > > (pgsigna

Re: [PATCHES] SIGPIPE handling

2003-11-16 Thread Kurt Roeckx
On Sun, Nov 16, 2003 at 12:56:10PM +0100, Manfred Spraul wrote: > Hi, > > attached is an update of my automatic sigaction patch: I've moved the > actual sigaction calls into pqsignal.c and added a helper function > (pgsignalinquire(signo)). I couldn't remove the include from > fe-connect.c: it

Re: [PATCHES] SIGPIPE handling

2003-11-16 Thread Manfred Spraul
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-

Re: [PATCHES] SIGPIPE handling

2003-11-16 Thread Tom Lane
Bruce Momjian <[EMAIL PROTECTED]> writes: > Is running the rest of the > application with SIGPIPE <= SIG_IGN a problem? That is NOT an acceptable thing for a library to do. regards, tom lane ---(end of broadcast)--- TIP 1: s

Re: [PATCHES] SIGPIPE handling

2003-11-16 Thread Bruce Momjian
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

Re: [PATCHES] SIGPIPE handling, take two.

2003-11-11 Thread Tom Lane
Manfred Spraul <[EMAIL PROTECTED]> writes: > ... But the SIG_IGN/restore > sequence affects the whole app - PQconnectdb calls would result in > randomly dropped SIGPIPE signals. Good point. AFAICS we lose anyway if we don't have sigaction() available, but hopefully any multithreaded platform ha

Re: [PATCHES] SIGPIPE handling, take two.

2003-11-11 Thread Manfred Spraul
Tom Lane wrote: I don't think we need to complicate pqsignal's API for this. Instead we'd better document that SIGPIPE handling has to be set up and kept stable before doing any libpq operations in a multithread app. Not reliable. An app could install it's own signal handler and block SIGPIPE

Re: [PATCHES] SIGPIPE handling, take two.

2003-11-11 Thread Tom Lane
Manfred Spraul <[EMAIL PROTECTED]> writes: > What about multithreaded apps? > old = pgsignal(SIPEPIPE, SIG_IGN); > ** another thread calls sigaction(SIGPIPE,,); > pgsignal(SIGPIPE, old); > And the signal state is corrupted. If other threads are changing the signal state mid-flight, we are

Re: [PATCHES] SIGPIPE handling, take two.

2003-11-11 Thread Manfred Spraul
Tom Lane wrote: Bruce Momjian <[EMAIL PROTECTED]> writes: I think this is the patch I like. The #if coding is messy and unnecessary. You could do the test as per the non-POSIX variant using two calls of pqsignal(), and not have any system dependence here, nor a need for . What about mu

Re: [PATCHES] SIGPIPE handling, take two.

2003-11-11 Thread Tom Lane
Oh, forgot to mention ... Bruce Momjian <[EMAIL PROTECTED]> writes: > 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. No, because this patch does not have any global effect on

Re: [PATCHES] SIGPIPE handling, take two.

2003-11-11 Thread Tom Lane
Bruce Momjian <[EMAIL PROTECTED]> writes: > I think this is the patch I like. The #if coding is messy and unnecessary. You could do the test as per the non-POSIX variant using two calls of pqsignal(), and not have any system dependence here, nor a need for . regards, tom

Re: [PATCHES] SIGPIPE handling, take two.

2003-11-11 Thread Gaetano Mendola
Gaetano Mendola wrote: Bruce Momjian wrote: 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 o

Re: [PATCHES] SIGPIPE handling, take two.

2003-11-11 Thread Gaetano Mendola
Bruce Momjian wrote: 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, a

Re: [PATCHES] SIGPIPE handling, take two.

2003-11-10 Thread Bruce Momjian
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. Base

Re: [PATCHES] sigpipe handling

2003-11-10 Thread Bruce Momjian
Sorry, but this just seems overdesigned. I liked Tom's suggestion: > One possibility that comes to mind is simply to test whether the SIGPIPE > handler is already SIG_IGN before we munge it. Ideally we'd do that > once when the conn object is created, but even if it had to be done more > often,