Date: Mon, 17 Jun 2002 02:07:30 -0700 From: Terry Lambert <tlambert2@mindspring.com> To: Alfred Perlstein <bright@mu.org> Cc: Michael Smith <msmith@mass.dis.org>, Giorgos Keramidas <keramida@ceid.upatras.gr>, Mario Sergio Fujikawa Ferreira <lioux@freebsd.org>, Garance A Drosihn <drosih@rpi.edu>, FreeBSD-arch@freebsd.org, msmith@freebsd.org Subject: Re: Adding SO_NOSIGPIPE to -STABLE/-CURRENT Message-ID: <3D0DA6D2.39A8C06F@mindspring.com> References: <3D0D904D.4752ADD4@mindspring.com> <200206170747.g5H7l3uJ001188@mass.dis.org> <20020617081320.GG67925@elvis.mu.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Alfred Perlstein wrote:
[ ... ]
Excuse me. Are you in the same discussion I'm in?
Please *read* Mario's patch; specifically:
--- sys/kern/sys_generic.c.orig Mon Feb 25 19:22:18 2002
+++ sys/kern/sys_generic.c Mon Feb 25 19:22:55 2002
@@ -409,7 +409,8 @@
if (auio.uio_resid != cnt && (error == ERESTART ||
error == EINTR || error == EWOULDBLOCK))
error = 0;
- if (error == EPIPE)
+ /* The socket layer handles SIGPIPE */
+ if (error == EPIPE && fp->f_type != DTYPE_SOCKET)
psignal(p, SIGPIPE);
}
cnt -= auio.uio_resid;
Turns off *all* socket-related SIGPIPE as a result of write(2)
and writev(2).
The other part of the patch:
--- sys/kern/uipc_syscalls.c.orig Mon Feb 25 19:59:54 2002
+++ sys/kern/uipc_syscalls.c Mon Feb 25 20:01:48 2002
@@ -586,7 +586,8 @@
if (auio.uio_resid != len && (error == ERESTART ||
error == EINTR || error == EWOULDBLOCK))
error = 0;
- if (error == EPIPE)
+ /* Generation of SIGPIPE can be controlled per socket */
+ if (error == EPIPE && !(so->so_options & SO_NOSIGPIPE))
psignal(p, SIGPIPE);
}
if (error == 0)
Is *only* effective in the send(2), sendto(2), and sendmsg(2)
cases.
In other words, it turns the SIGPIPE off in a code path, makes
it switchable in a second code path, and thereafter provides no
mechanism whereby the SISGPIPE may be turned back on in the first
code path.
The /socket/ is not available in the write path, the /descriptor/
is; to convert it from a /descriptor/ to a /socket/, you would
have to do a lot of tests and dereferencing (nominally OK, since
it's an already expensive error path, but hard to do without
dragging in most of the socket headers into kern/sys_generic.c).
Therefore, my *initial* post in this thread suggested that, if it
be done at all, it be done as a /descriptor/ option, via fcntl,
rather than as a /socket/ option, via setsockopt(). I was not
aware that people has used up all the available fcntl() commands.
Unless you fix the CLEVERLY HIDDEN BREAKAGE it introduces in
dofilewrite() on sockets in sys_generic.c, I think it's a BAD IDEA
to commit the patch.
I *also* think that if this option is set on a listening socket,
that, if it is implemented in this way, it should be inheritted
by each accepted socket. If the bad idea must go forward, it
should at least be implemented correctly.
The obvious reasoning for this is that there is not an fd available
on which to set the option for the new connection until after the
accept(2) is called to accept the connection. Since the *entire*
point of this patch *must* be to save the system call to check to
see if a descriptor is writeable, OR to save the two system calls
that disabling and reenabling the SIGPIPE around a write to close
the tiny check-then-write race window, saving additional system
calls on each socket opened to set the option is also good.
Looking at it from a general standpoint, I'd *still* like to see
some code that people are going to be running on FreeBSD that
won't run without it.
IF the option is committed because of the "compatability" argument,
I'd like to see it named the same thing as at least one other OS,
to avoid "#ifdef FreeBSD"; if that's not going to happen, then you
might as well put better code in the #ifdef FreeBSD", instead.
If I seem to think that this promotes signal semantics that I think
ought to be determined by the system, well, here's Mario's original
rationale for the change in the first place:
| For those that might find it useful, here go a
| simple scenario not easily reproduceable without SO_NOSIGPIPE:
| multi-threaded client, some threads want to handle SIGPIPE,
| others not.
...in other words, to control the threads signal semantics more
closely than is normally permitted by the POSIX definition of
signals.
If I sound like I think this is a slippery slope -- I DO. I can
see *at least* wanting to so the same for any signal that could
result from a "write" that has a default of "terminate process".
I could also see wanting to set the same for SIGURG -- in case
the user establishes a SIGURG handler outside the context of
the library and/or threaded application under discussion. It
is *not* just "terminate process" signal actions that you have
to worry about, it's also any signal whose default action is
"discard signal", and which might have been overridden by the
user.
-- Terry
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-arch" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3D0DA6D2.39A8C06F>