svn commit: r196460 - head/sys/kern

[Jilles Tjoelker]

On Sun, Aug 23, 2009 at 12:44:15PM +0000, Konstantin Belousov wrote:
> Author: kib
> Date: Sun Aug 23 12:44:15 2009
> New Revision: 196460
> URL: http://svn.freebsd.org/changeset/base/196460

> Log:
>   Fix the conformance of poll(2) for sockets after r195423 by
>   returning POLLHUP instead of POLLIN for several cases. Now, the
>   tools/regression/poll results for FreeBSD are closer to that of the
>   Solaris and Linux.

>   Also, improve the POSIX conformance by explicitely clearing POLLOUT
>   when POLLHUP is reported in pollscan(), making the fix global.

>   Submitted by:	bde
>   Reviewed by:	rwatson
>   MFC after:	1 week

> Modified:
>   head/sys/kern/sys_generic.c
>   head/sys/kern/uipc_socket.c

> Modified: head/sys/kern/sys_generic.c
> ==============================================================================
> --- head/sys/kern/sys_generic.c	Sun Aug 23 12:23:24 2009	(r196459)
> +++ head/sys/kern/sys_generic.c	Sun Aug 23 12:44:15 2009	(r196460)
> @@ -1228,6 +1228,13 @@ pollscan(td, fds, nfd)
>  				selfdalloc(td, fds);
>  				fds->revents = fo_poll(fp, fds->events,
>  				    td->td_ucred, td);
> +				/*
> +				 * POSIX requires POLLOUT to be never
> +				 * set simultaneously with POLLHUP.
> +				 */
> +				if ((fds->revents & POLLHUP) != 0)
> +					fds->revents &= ~POLLOUT;
> +
>  				if (fds->revents != 0)
>  					n++;
>  			}

This looks useful.

> Modified: head/sys/kern/uipc_socket.c
> ==============================================================================
> --- head/sys/kern/uipc_socket.c	Sun Aug 23 12:23:24 2009	(r196459)
> +++ head/sys/kern/uipc_socket.c	Sun Aug 23 12:44:15 2009	(r196460)
> @@ -2898,13 +2898,11 @@ sopoll_generic(struct socket *so, int ev
>  		if (so->so_oobmark || (so->so_rcv.sb_state & SBS_RCVATMARK))
>  			revents |= events & (POLLPRI | POLLRDBAND);
>  
> -	if ((events & POLLINIGNEOF) == 0) {
> -		if (so->so_rcv.sb_state & SBS_CANTRCVMORE) {
> -			revents |= events & (POLLIN | POLLRDNORM);
> -			if (so->so_snd.sb_state & SBS_CANTSENDMORE)
> -				revents |= POLLHUP;
> -		}
> -	}
> +	if ((events & POLLINIGNEOF) == 0)
> +		if (so->so_rcv.sb_state & SBS_CANTRCVMORE)
> +			revents |= POLLHUP;
> +	if (so->so_snd.sb_state & SBS_CANTSENDMORE)
> +		revents |= POLLHUP;
>  
>  	if (revents == 0) {
>  		if (events & (POLLIN | POLLPRI | POLLRDNORM | POLLRDBAND)) {

This sets POLLHUP when either of the directions has shut down, instead
of the entire connection. This breaks use of select and poll with
shutdown(2):

* sending some data, shutdown(SHUT_WR) then polling for input
* (the other side for the above) receiving some data, getting EOF on
  that direction then polling for output
* a paranoid HTTP-like server that wants to wait for the client to close
  the read end after shutting down the write end

Either some data is lost or the program busy-waits for the data.

I think the POLLHUP setting before this commit was correct for sockets:
POLLHUP is set if both directions are closed/error. An EOF that only
affects one direction sets the corresponding POLLIN/POLLOUT so that the
EOF becomes known but the other direction can still be used normally.

(The POSIX spec explicitly describes something like this for POLLIN
(zero length message) although it erroneously restricts it to STREAMS
files only; the POLLOUT case has to be derived from the fact that the
read end should still work normally but the EOF should be notified.)

I think poll on fifos should instead be fixed by closing the
half-connection corresponding to writing from fi_readsock to
fi_writesock. I have tried this out, see attached patch. With the patch,
pipepoll only gives "expected POLLHUP; got POLLIN | POLLHUP" errors and
an error in fifo case 6b caused by our distinction between new and old
readers.

tools/regression/poll does not test shutdown(2) interaction, so it does
not find this problem.

-- 
Jilles Tjoelker
-------------- next part --------------
A non-text attachment was scrubbed...
Name: poll-fifo.patch
Type: text/x-diff
Size: 1258 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/svn-src-head/attachments/20090823/cf515a7d/poll-fifo.bin