Re: [PATCHES] [HACKERS] libpq and psql not on same page about SIGPIPE
Tom Lane wrote: > Bruce Momjian <[EMAIL PROTECTED]> writes: > > OK, patch applied. I also documented an optimmization we might make > > leter in fe-secure.c: > > > n = send(conn->sock, ptr, len, 0); > > /* > > * Possible optimization: if sigpending() turns out to be an > > * expensive operation, we can set sigpipe_pending = 'true' > > * here if errno != EPIPE, avoiding a sigpending call. > > */ > > I went ahead and did that (or a cleaner equivalent) because I think it's > important that we not risk clearing events we shouldn't clear. In the > normal path where we don't get EPIPE, it's now certain that secure_write > won't have any side effects on the application state; saving a kernel > call is a free bonus. The fe-print.c code is a bit more problematic > but I tend to think it's vestigial anyway. Agreed. > Per our phone discussion this morning, this code should be solid except > in the case where the C library is able to queue multiple pending > SIGPIPE events. Like you, I'm dubious that that exists in the real > world, or that anybody could cope with it if it did. (Example: if > someone blocks SIGPIPE and calls a complex function like fprintf(), how > could he be certain that only one SIGPIPE event had been queued before > it returned?) Great. Right now the code path is only two pthread_sigmask() calls. That has got to be quite fast, I bet. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [HACKERS] libpq and psql not on same page about SIGPIPE
Bruce Momjian <[EMAIL PROTECTED]> writes: > OK, patch applied. I also documented an optimmization we might make > leter in fe-secure.c: > n = send(conn->sock, ptr, len, 0); > /* > * Possible optimization: if sigpending() turns out to be an > * expensive operation, we can set sigpipe_pending = 'true' > * here if errno != EPIPE, avoiding a sigpending call. > */ I went ahead and did that (or a cleaner equivalent) because I think it's important that we not risk clearing events we shouldn't clear. In the normal path where we don't get EPIPE, it's now certain that secure_write won't have any side effects on the application state; saving a kernel call is a free bonus. The fe-print.c code is a bit more problematic but I tend to think it's vestigial anyway. Per our phone discussion this morning, this code should be solid except in the case where the C library is able to queue multiple pending SIGPIPE events. Like you, I'm dubious that that exists in the real world, or that anybody could cope with it if it did. (Example: if someone blocks SIGPIPE and calls a complex function like fprintf(), how could he be certain that only one SIGPIPE event had been queued before it returned?) regards, tom lane ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [HACKERS] libpq and psql not on same page about SIGPIPE
Tom Lane wrote: > Bruce Momjian <[EMAIL PROTECTED]> writes: > > Tom Lane wrote: > >> No, that's wrong: if there is a pending SIGPIPE that belongs to the > >> outer app, you'd clear it. > > > True, but I documented that in the patch. > > A documented bug is still a bug. The entire point of this change is to > not interfere with the behavior of the surrounding app, at least not > more than we absolutely have to; and we do not have to clear SIGPIPEs > that are certainly not ours. I am working on a new version that doesn't have this problem. Will post soon. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] [HACKERS] libpq and psql not on same page about SIGPIPE
Bruce Momjian <[EMAIL PROTECTED]> writes: > Tom Lane wrote: >> No, that's wrong: if there is a pending SIGPIPE that belongs to the >> outer app, you'd clear it. > True, but I documented that in the patch. A documented bug is still a bug. The entire point of this change is to not interfere with the behavior of the surrounding app, at least not more than we absolutely have to; and we do not have to clear SIGPIPEs that are certainly not ours. regards, tom lane ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] [HACKERS] libpq and psql not on same page about SIGPIPE
Tom Lane wrote:
> Bruce Momjian <[EMAIL PROTECTED]> writes:
> > Yes, that is the logic in my patch, except that I don't check errno, I
> > just call sigpending().
>
> No, that's wrong: if there is a pending SIGPIPE that belongs to the
> outer app, you'd clear it.
True, but I documented that in the patch.
> > There are a few writes and it seemed impossible
> > to check them all.
>
> Hmm? There is only one place this needs to be done, namely
> pqsecure_write.
Look also in fe-print.c. Looks like a lot of popen writes in there.
I can do it but it will be harder.
> BTW, have you posted the patch yet or are you still working on it?
> The mail server seems a bit flaky today ...
OK, patch attached. I already sent it but who knows what happened to
it.
--
Bruce Momjian| http://candle.pha.pa.us
[EMAIL PROTECTED] | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup.| Newtown Square, Pennsylvania 19073
Index: configure
===
RCS file: /cvsroot/pgsql/configure,v
retrieving revision 1.409
diff -c -c -r1.409 configure
*** configure 30 Nov 2004 06:13:03 - 1.409
--- configure 1 Dec 2004 22:53:58 -
***
*** 17431,17436
--- 17431,17448
fi
HAVE_POSIX_SIGNALS=$pgac_cv_func_posix_signals
+ if test "$pgac_cv_func_posix_signals" != yes -a "$enable_thread_safety" =
yes; then
+ { { echo "$as_me:$LINENO: error:
+ *** Thread-safety requires POSIX signals, which are not supported by your
+ *** operating system.
+ " >&5
+ echo "$as_me: error:
+ *** Thread-safety requires POSIX signals, which are not supported by your
+ *** operating system.
+ " >&2;}
+{ (exit 1); exit 1; }; }
+ fi
+
if test $ac_cv_func_fseeko = yes; then
# Check whether --enable-largefile or --disable-largefile was given.
if test "${enable_largefile+set}" = set; then
Index: configure.in
===
RCS file: /cvsroot/pgsql/configure.in,v
retrieving revision 1.387
diff -c -c -r1.387 configure.in
*** configure.in30 Nov 2004 06:13:04 - 1.387
--- configure.in1 Dec 2004 22:54:11 -
***
*** 1174,1179
--- 1174,1186
PGAC_FUNC_POSIX_SIGNALS
+ if test "$pgac_cv_func_posix_signals" != yes -a "$enable_thread_safety" =
yes; then
+ AC_MSG_ERROR([
+ *** Thread-safety requires POSIX signals, which are not supported by your
+ *** operating system.
+ ])
+ fi
+
if test $ac_cv_func_fseeko = yes; then
AC_SYS_LARGEFILE
fi
Index: doc/src/sgml/libpq.sgml
===
RCS file: /cvsroot/pgsql/doc/src/sgml/libpq.sgml,v
retrieving revision 1.169
diff -c -c -r1.169 libpq.sgml
*** doc/src/sgml/libpq.sgml 27 Nov 2004 21:56:04 - 1.169
--- doc/src/sgml/libpq.sgml 1 Dec 2004 22:54:45 -
***
*** 3955,3975
! libpq must ignore SIGPIPE signals
! generated internally by send() calls to backend processes.
! When PostgreSQL is configured without
! --enable-thread-safety, libpq sets
! SIGPIPE to SIG_IGN before each
! send() call and restores the original signal handler after
! completion. When --enable-thread-safety is used,
! libpq installs its own SIGPIPE handler
! before the first database connection. This handler uses thread-local
! storage to determine if a SIGPIPE signal has been generated
! by a libpq send(). If an application wants to install
! its own SIGPIPE signal handler, it should call
! PQinSend() to determine if it should ignore the
! SIGPIPE signal. This function is available in both
! thread-safe and non-thread-safe versions of libpq.
--- 3955,3964
! libpq blocks and discards SIGPIPE
! signals generated internally by send() calls to the backend
! process. Therefore, applications should not expect blocked SIGPIPE
! signals to remain across libpq function calls.
Index: doc/src/sgml/ref/copy.sgml
===
RCS file: /cvsroot/pgsql/doc/src/sgml/ref/copy.sgml,v
retrieving revision 1.60
diff -c -c -r1.60 copy.sgml
*** doc/src/sgml/ref/copy.sgml 27 Nov 2004 21:56:05 - 1.60
--- doc/src/sgml/ref/copy.sgml 1 Dec 2004 22:54:57 -
***
*** 3,8
--- 3,9
PostgreSQL documentation
-->
+
COPY
Index: src/interfaces/libpq/fe-connect.c
===
RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.289
diff -c -c -r1.289 fe-connect.c
*** src/interfaces/libpq/fe-connect.c 30 Oct 2004 23:11:26 - 1.289
--- src/interfaces/libpq/fe-connect.c 1 Dec 2004 22:55:18 -
***
*** 865,879
const char *node = NULL;
int ret;
- #ifdef ENABLE_THREAD_SAFETY
- #ifndef WIN32
-
