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>