From owner-freebsd-bugs@FreeBSD.ORG Wed Mar 22 21:21:07 2006 Return-Path: X-Original-To: freebsd-bugs@hub.freebsd.org Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 0624E16A433 for ; Wed, 22 Mar 2006 21:21:07 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id 342FD43DF4 for ; Wed, 22 Mar 2006 21:20:17 +0000 (GMT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.13.4/8.13.4) with ESMTP id k2MLKG5T034314 for ; Wed, 22 Mar 2006 21:20:16 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.13.4/8.13.4/Submit) id k2MLKG2C034313; Wed, 22 Mar 2006 21:20:16 GMT (envelope-from gnats) Date: Wed, 22 Mar 2006 21:20:16 GMT Message-Id: <200603222120.k2MLKG2C034313@freefall.freebsd.org> To: freebsd-bugs@FreeBSD.org From: Oliver Fromme Cc: Subject: Re: kern/94772: FIFOs (named pipes) + select() == broken X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Oliver Fromme List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 22 Mar 2006 21:21:07 -0000 The following reply was made to PR kern/94772; it has been noted by GNATS. From: Oliver Fromme To: bde@zeta.org.au (Bruce Evans) Cc: bug-followup@freebsd.org Subject: Re: kern/94772: FIFOs (named pipes) + select() == broken Date: Wed, 22 Mar 2006 22:12:32 +0100 (CET) Hi, Sorry for replying to myself, but there are a few new things ... Oliver Fromme wrote: > Bruce Evans wrote: > > On Tue, 21 Mar 2006, Oliver Fromme wrote: > > > When a FIFO had been opened for reading and the writing > > > process closes it, the reading process blocks in select(), > > > even though the descriptor is ready for read(). If the > > > select() call is omitted, read() returns 0 immediately > > > indicating EOF. But as soon as you use select(), it blocks > > > and there is no chance to detect the EOF condition. > > > > See also: > > > > PR 76125 (about the same bug) > > PR 76144 (about a related bug in poll()) > > PR 53447 (about a another or the same related bug in poll()) > > PR 34020 (about the inverse of the bug (return on non-EOF) for select() > > and poll()) The first one (76125) seems completely unrelated. Typo? > (By the way, DEC UNIX 4.0D _does_ have a bug: If the FIFO > has O_NONBLOCK set and no writer has opened the FIFO, then > select() doesn't block. Actually, it's not a bug. I've read SUSv3 wrong. That behaviour is perfectly fine. In fact, SUSv3 (a.k.a. POSIX-2001) requires that select() doesn't block in that case, and the behaviour of select() and poll() must be independet of whether O_NONBLOCK is set or not. > > Fix (?): > > - actually implement returning POLLHUP in sopoll() and other places. Return > > POLLHUP but not POLLIN for the pure-EOF case. Return POLLIN* | POLLHUP > > for EOF-with-data. > > - remove POLLINIGNEOF and associated complications in sopoll(), fifo_poll() > > and > > - change selscan() to check for POLLHUP so that POLLIN, POLLIN | POLLHUP > > and POLLHUP all act the same for select() > > - remove POLLHUP from the comment in selscan(). Fix the rest of this > > comment or remove it (most backends are too broken to return poll flags > > if appropriate, and the comment only mentions one of the other poll flags > > that selscan() ignores) > > - remove the corresponding comment in pollscan() since it is wrong and says > > nothing relevant (pollscan() just accepts whatever flags the backends set). > > I would be happy to help out, but I'm not familiar with > that part of the kernel code. Well, now (a few hours later) I'm a little bit more familiar with it. Patch attached below, and also available from this URL: http://www.secnetix.de/~olli/tmp/fifodiff.txt With that patch, my own test program (the one from the top of this PR) runs successfully, i.e. EOF is recognized correctly in all cases that I have tested (with and without select()), and it also behaves standard-compliant when O_NONBLOCK is used (see above). Also, with that patch applied, your test program runs successfully (without producing any output). I've tested everything under RELENG_6 (cvsupped today). I've also had a look at the Current sources, and the relevant parts don't seem to be any different, so the diff should be applicable to HEAD as well. The patch addresses the following things: - implement POLLHUP in sopoll(). - remove POLLINIGNEOF. - selscan() doesn't need to be patched: it already works as expected when fo_poll() returns POLLHUP. - I don't think the comment is entirely incorrect, but I'm not sure, so I left it alone. Please give this patch a try and let me know if there are any problems (functional, style, whatever). As far as I can tell, the patch fixes the existing bugs with FIFOs + select()/poll(), so I would be happy to see it committed to Current and RELENG_6. (Maybe even in time for the release of 6.1?) Best regards Oliver --- src/sys/fs/fifofs/fifo_vnops.c.orig Tue Mar 21 09:42:32 2006 +++ src/sys/fs/fifofs/fifo_vnops.c Wed Mar 22 18:49:27 2006 @@ -661,31 +661,11 @@ int levents, revents = 0; fip = fp->f_data; - levents = events & - (POLLIN | POLLINIGNEOF | POLLPRI | POLLRDNORM | POLLRDBAND); + levents = events & (POLLIN | POLLPRI | POLLRDNORM | POLLRDBAND); if ((fp->f_flag & FREAD) && levents) { - /* - * If POLLIN or POLLRDNORM is requested and POLLINIGNEOF is - * not, then convert the first two to the last one. This - * tells the socket poll function to ignore EOF so that we - * block if there is no writer (and no data). Callers can - * set POLLINIGNEOF to get non-blocking behavior. - */ - if (levents & (POLLIN | POLLRDNORM) && - !(levents & POLLINIGNEOF)) { - levents &= ~(POLLIN | POLLRDNORM); - levents |= POLLINIGNEOF; - } - filetmp.f_data = fip->fi_readsock; filetmp.f_cred = cred; revents |= soo_poll(&filetmp, levents, cred, td); - - /* Reverse the above conversion. */ - if ((revents & POLLINIGNEOF) && !(events & POLLINIGNEOF)) { - revents |= (events & (POLLIN | POLLRDNORM)); - revents &= ~POLLINIGNEOF; - } } levents = events & (POLLOUT | POLLWRNORM | POLLWRBAND); if ((fp->f_flag & FWRITE) && levents) { --- src/sys/kern/uipc_socket.c.orig Wed Dec 28 19:05:13 2005 +++ src/sys/kern/uipc_socket.c Wed Mar 22 18:44:08 2006 @@ -2036,23 +2036,26 @@ if (soreadable(so)) revents |= events & (POLLIN | POLLRDNORM); - if (events & POLLINIGNEOF) - if (so->so_rcv.sb_cc >= so->so_rcv.sb_lowat || - !TAILQ_EMPTY(&so->so_comp) || so->so_error) - revents |= POLLINIGNEOF; - - if (events & (POLLOUT | POLLWRNORM)) - if (sowriteable(so)) - revents |= events & (POLLOUT | POLLWRNORM); + if (events & (POLLOUT | POLLWRNORM) && sowriteable(so)) + revents |= events & (POLLOUT | POLLWRNORM); + else { + /* + * POLLOUT and POLLHUP shall not both be set. + * Therefore check only for POLLHUP if POLLOUT + * has not been set. (Note that POLLHUP need + * not be in events; it's always checked.) + */ + if (so->so_rcv.sb_state & SBS_CANTRCVMORE && + so->so_rcv.sb_cc == 0) + revents |= POLLHUP; + } if (events & (POLLPRI | POLLRDBAND)) if (so->so_oobmark || (so->so_rcv.sb_state & SBS_RCVATMARK)) revents |= events & (POLLPRI | POLLRDBAND); if (revents == 0) { - if (events & - (POLLIN | POLLINIGNEOF | POLLPRI | POLLRDNORM | - POLLRDBAND)) { + if (events & (POLLIN | POLLPRI | POLLRDNORM | POLLRDBAND)) { selrecord(td, &so->so_rcv.sb_sel); so->so_rcv.sb_flags |= SB_SEL; } --- src/sys/sys/poll.h.orig Wed Jul 10 06:47:25 2002 +++ src/sys/sys/poll.h Wed Mar 22 18:41:03 2006 @@ -66,11 +66,6 @@ #define POLLRDBAND 0x0080 /* OOB/Urgent readable data */ #define POLLWRBAND 0x0100 /* OOB/Urgent data can be written */ -#if __BSD_VISIBLE -/* General FreeBSD extension (currently only supported for sockets): */ -#define POLLINIGNEOF 0x2000 /* like POLLIN, except ignore EOF */ -#endif - /* * These events are set if they occur regardless of whether they were * requested. -- Oliver Fromme, secnetix GmbH & Co. KG, Marktplatz 29, 85567 Grafing Dienstleistungen mit Schwerpunkt FreeBSD: http://www.secnetix.de/bsd Any opinions expressed in this message may be personal to the author and may not necessarily reflect the opinions of secnetix in any way. "I learned Java 3 years before Python. It was my language of choice. It took me two weekends with Python before I was more productive with it than with Java." -- Anthony Roberts