Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 24 Aug 2009 01:49:02 +0300
From:      Kostik Belousov <kostikbel@gmail.com>
To:        Jilles Tjoelker <jilles@stack.nl>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r196460 - head/sys/kern
Message-ID:  <20090823224902.GK9623@deviant.kiev.zoral.com.ua>
In-Reply-To: <20090823213759.GA55039@stack.nl>
References:  <200908231244.n7NCiFgc061095@svn.freebsd.org> <20090823213759.GA55039@stack.nl>

next in thread | previous in thread | raw e-mail | index | archive | help

--NSWmqrgMASeejWAY
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Sun, Aug 23, 2009 at 11:38:00PM +0200, Jilles Tjoelker wrote:
> 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
>=20
> > 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.
>=20
> >   Also, improve the POSIX conformance by explicitely clearing POLLOUT
> >   when POLLHUP is reported in pollscan(), making the fix global.
>=20
> >   Submitted by:	bde
> >   Reviewed by:	rwatson
> >   MFC after:	1 week
=2E..
> > Modified: head/sys/kern/uipc_socket.c
> > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D
> > --- 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 |=3D events & (POLLPRI | POLLRDBAND);
> > =20
> > -	if ((events & POLLINIGNEOF) =3D=3D 0) {
> > -		if (so->so_rcv.sb_state & SBS_CANTRCVMORE) {
> > -			revents |=3D events & (POLLIN | POLLRDNORM);
> > -			if (so->so_snd.sb_state & SBS_CANTSENDMORE)
> > -				revents |=3D POLLHUP;
> > -		}
> > -	}
> > +	if ((events & POLLINIGNEOF) =3D=3D 0)
> > +		if (so->so_rcv.sb_state & SBS_CANTRCVMORE)
> > +			revents |=3D POLLHUP;
> > +	if (so->so_snd.sb_state & SBS_CANTSENDMORE)
> > +		revents |=3D POLLHUP;
> > =20
> >  	if (revents =3D=3D 0) {
> >  		if (events & (POLLIN | POLLPRI | POLLRDNORM | POLLRDBAND)) {
>=20
> 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):
>=20
> * 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
>=20
> Either some data is lost or the program busy-waits for the data.
Exactly this situation concerned me and Robert.

>=20
> 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.
>=20
> (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.)
>=20
> 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.
This sounds right.
Please go ahead.

>=20
> tools/regression/poll does not test shutdown(2) interaction, so it does
> not find this problem.
>=20
> --=20
> Jilles Tjoelker



--NSWmqrgMASeejWAY
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (FreeBSD)

iEYEARECAAYFAkqRx10ACgkQC3+MBN1Mb4hgBwCfTY08i+d4EXvg/r+7J/QTqyTI
whYAoOx7xZlC5rYB6bKMQJJXpOudmsEP
=z2Kx
-----END PGP SIGNATURE-----

--NSWmqrgMASeejWAY--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20090823224902.GK9623>