Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src - fifo_poll(): If the last writer has disappeared, detect th...



details:   https://anonhg.NetBSD.org/src/rev/950cfc6930bd
branches:  trunk
changeset: 1023914:950cfc6930bd
user:      thorpej <thorpej%NetBSD.org@localhost>
date:      Sat Oct 02 02:07:41 2021 +0000

description:
- fifo_poll(): If the last writer has disappeared, detect this and return
  POLLHUP, per POSIX.
- fifo_close(): Use the new fifo_socantrcvmore(), which is like the
  garden-variety socantrcvmore(), except it specifies POLL_HUP rather
  than POLL_IN (so the correct code for SIGIO is sent).
- sowakeup(): Allow POLL_HUP as a code (notifies poll'ers with POLLHUP).
- Add test cases for correct POLLHUP behavior with FIFOs.

Fixes PR kern/56429.

diffstat:

 sys/kern/uipc_socket2.c        |   25 ++++++++-
 sys/miscfs/fifofs/fifo_vnops.c |   93 +++++++++++++++++++++++++++++------
 tests/lib/libc/sys/t_poll.c    |  107 ++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 203 insertions(+), 22 deletions(-)

diffs (truncated from 320 to 300 lines):

diff -r 411c071d764f -r 950cfc6930bd sys/kern/uipc_socket2.c
--- a/sys/kern/uipc_socket2.c   Fri Oct 01 22:42:28 2021 +0000
+++ b/sys/kern/uipc_socket2.c   Sat Oct 02 02:07:41 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: uipc_socket2.c,v 1.139 2021/03/04 01:35:31 msaitoh Exp $       */
+/*     $NetBSD: uipc_socket2.c,v 1.140 2021/10/02 02:07:41 thorpej Exp $       */
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: uipc_socket2.c,v 1.139 2021/03/04 01:35:31 msaitoh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: uipc_socket2.c,v 1.140 2021/10/02 02:07:41 thorpej Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_ddb.h"
@@ -555,10 +555,27 @@
        KASSERT(solocked(so));
        KASSERT(sb->sb_so == so);
 
-       if (code == POLL_IN)
+       switch (code) {
+       case POLL_IN:
                band = POLLIN|POLLRDNORM;
-       else
+               break;
+
+       case POLL_OUT:
                band = POLLOUT|POLLWRNORM;
+               break;
+
+       case POLL_HUP:
+               band = POLLHUP;
+               break;
+
+       default:
+               band = 0;
+#ifdef DIAGNOSTIC
+               printf("bad siginfo code %d in socket notification.\n", code);
+#endif 
+               break;
+       }
+
        sb->sb_flags &= ~SB_NOTIFY;
        selnotify(&sb->sb_sel, band, NOTE_SUBMIT);
        cv_broadcast(&sb->sb_cv);
diff -r 411c071d764f -r 950cfc6930bd sys/miscfs/fifofs/fifo_vnops.c
--- a/sys/miscfs/fifofs/fifo_vnops.c    Fri Oct 01 22:42:28 2021 +0000
+++ b/sys/miscfs/fifofs/fifo_vnops.c    Sat Oct 02 02:07:41 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: fifo_vnops.c,v 1.86 2021/09/29 13:15:45 thorpej Exp $  */
+/*     $NetBSD: fifo_vnops.c,v 1.87 2021/10/02 02:07:41 thorpej Exp $  */
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: fifo_vnops.c,v 1.86 2021/09/29 13:15:45 thorpej Exp $");
+__KERNEL_RCSID(0, "$NetBSD: fifo_vnops.c,v 1.87 2021/10/02 02:07:41 thorpej Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -337,24 +337,69 @@
        struct vop_poll_args /* {
                struct vnode    *a_vp;
                int             a_events;
-               struct lwp      *a_l;
        } */ *ap = v;
-       struct socket   *so;
-       int             revents;
+       struct socket   *rso = ap->a_vp->v_fifoinfo->fi_readsock;
+       struct socket   *wso = ap->a_vp->v_fifoinfo->fi_writesock;
+       struct socket   *lso = NULL;
+       int             events;
 
-       revents = 0;
-       if (ap->a_events & (POLLIN | POLLPRI | POLLRDNORM | POLLRDBAND)) {
-               so = ap->a_vp->v_fifoinfo->fi_readsock;
-               if (so)
-                       revents |= sopoll(so, ap->a_events);
+       /*
+        * N.B. We're using a slightly different naming convention
+        * for these variables that most poll handlers.
+        */
+       int             revents = 0;
+       int             wevents = 0;
+
+       if (rso != NULL) {
+               lso = rso;
+       } else if (wso != NULL) {
+               lso = wso;
        }
-       if (ap->a_events & (POLLOUT | POLLWRNORM | POLLWRBAND)) {
-               so = ap->a_vp->v_fifoinfo->fi_writesock;
-               if (so)
-                       revents |= sopoll(so, ap->a_events);
+
+       if (lso == NULL) {
+               /* No associated sockets -> no events to report. */
+               return 0;
        }
 
-       return (revents);
+       KASSERT(rso == NULL || lso->so_lock == rso->so_lock);
+       KASSERT(wso == NULL || lso->so_lock == wso->so_lock);
+
+       solock(lso);
+
+       if (rso != NULL) {
+               events = ap->a_events & (POLLIN | POLLRDNORM);
+               if (events != 0 && soreadable(rso)) {
+                       revents |= events;
+               }
+               if (rso->so_state & SS_CANTRCVMORE) {
+                       revents |= POLLHUP;
+               }
+               /*
+                * We always selrecord the read side here regardless
+                * of the caller's read interest because we need to
+                * action POLLHUP.
+                */
+               if (revents == 0) {
+                       selrecord(curlwp, &rso->so_rcv.sb_sel);
+                       rso->so_rcv.sb_flags |= SB_NOTIFY;
+               }
+       }
+
+       /* POSIX sez: POLLHUP and POLLOUT are mutually-exclusive. */
+       if (wso != NULL && (revents & POLLHUP) == 0) {
+               events = ap->a_events & (POLLOUT | POLLWRNORM);
+               if (events != 0 && sowritable(wso)) {
+                       wevents |= events;
+               }
+               if (wevents == 0 && events != 0) {
+                       selrecord(curlwp, &wso->so_snd.sb_sel);
+                       wso->so_snd.sb_flags |= SB_NOTIFY;
+               }
+       }
+
+       sounlock(lso);
+
+       return (revents | wevents);
 }
 
 static int
@@ -392,6 +437,20 @@
 }
 
 /*
+ * This is like socantrcvmore(), but we send the POLL_HUP code.
+ */
+static void
+fifo_socantrcvmore(struct socket *so)
+{
+       KASSERT(solocked(so));
+
+       so->so_state |= SS_CANTRCVMORE;
+       if (sb_notify(&so->so_rcv)) {
+               sowakeup(so, &so->so_rcv, POLL_HUP);
+       }
+}
+
+/*
  * Device close routine
  */
 /* ARGSUSED */
@@ -422,13 +481,13 @@
                }
                if (fip->fi_writers != 0) {
                        fip->fi_writers = 0;
-                       socantrcvmore(rso);
+                       fifo_socantrcvmore(rso);
                }
        } else {
                if ((ap->a_fflag & FREAD) && --fip->fi_readers == 0)
                        socantsendmore(wso);
                if ((ap->a_fflag & FWRITE) && --fip->fi_writers == 0)
-                       socantrcvmore(rso);
+                       fifo_socantrcvmore(rso);
        }
        if ((fip->fi_readers + fip->fi_writers) == 0) {
                sounlock(wso);
diff -r 411c071d764f -r 950cfc6930bd tests/lib/libc/sys/t_poll.c
--- a/tests/lib/libc/sys/t_poll.c       Fri Oct 01 22:42:28 2021 +0000
+++ b/tests/lib/libc/sys/t_poll.c       Sat Oct 02 02:07:41 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: t_poll.c,v 1.4 2020/07/17 15:34:16 kamil Exp $ */
+/*     $NetBSD: t_poll.c,v 1.5 2021/10/02 02:07:41 thorpej Exp $       */
 
 /*-
  * Copyright (c) 2011 The NetBSD Foundation, Inc.
@@ -29,6 +29,7 @@
  * POSSIBILITY OF SUCH DAMAGE.
  */
 
+#include <sys/stat.h>
 #include <sys/time.h>
 #include <sys/wait.h>
 
@@ -233,6 +234,107 @@
        ATF_REQUIRE_ERRNO(EINVAL, poll(&pfd, 1, -2) == -1);
 }
 
+static const char      fifo_path[] = "pollhup_fifo";
+
+static void
+fifo_support(void)
+{
+       errno = 0;
+       if (mkfifo(fifo_path, 0600) == 0) {
+               ATF_REQUIRE(unlink(fifo_path) == 0);
+               return;
+       }
+
+       if (errno == EOPNOTSUPP) {
+               atf_tc_skip("the kernel does not support FIFOs");
+       } else {
+               atf_tc_fail("mkfifo(2) failed");
+       }
+}
+
+ATF_TC_WITH_CLEANUP(fifo_hup1);
+ATF_TC_HEAD(fifo_hup1, tc)
+{
+       atf_tc_set_md_var(tc, "descr",
+           "Check POLLHUP behavior with fifos [1]");
+}
+
+ATF_TC_BODY(fifo_hup1, tc)
+{
+       struct pollfd pfd;
+       int rfd, wfd;
+
+       fifo_support();
+
+       ATF_REQUIRE(mkfifo(fifo_path, 0600) == 0);
+       ATF_REQUIRE((rfd = open(fifo_path, O_RDONLY | O_NONBLOCK)) >= 0);
+       ATF_REQUIRE((wfd = open(fifo_path, O_WRONLY)) >= 0);
+
+       memset(&pfd, 0, sizeof(pfd));
+       pfd.fd = rfd;
+       pfd.events = POLLIN;
+
+       (void)close(wfd);
+
+       ATF_REQUIRE(poll(&pfd, 1, 0) == 1);
+       ATF_REQUIRE((pfd.revents & POLLHUP) != 0);
+}
+
+ATF_TC_CLEANUP(fifo_hup1, tc)
+{
+       (void)unlink(fifo_path);
+}
+
+ATF_TC_WITH_CLEANUP(fifo_hup2);
+ATF_TC_HEAD(fifo_hup2, tc)
+{
+       atf_tc_set_md_var(tc, "descr",
+           "Check POLLHUP behavior with fifos [2]");
+}
+
+ATF_TC_BODY(fifo_hup2, tc)
+{
+       struct pollfd pfd;
+       int rfd, wfd;
+       pid_t pid;
+       struct timespec ts1, ts2;
+
+       fifo_support();
+
+       ATF_REQUIRE(mkfifo(fifo_path, 0600) == 0);
+       ATF_REQUIRE((rfd = open(fifo_path, O_RDONLY | O_NONBLOCK)) >= 0);
+       ATF_REQUIRE((wfd = open(fifo_path, O_WRONLY)) >= 0);
+
+       memset(&pfd, 0, sizeof(pfd));
+       pfd.fd = rfd;
+       pfd.events = POLLIN;
+
+       pid = fork();
+       ATF_REQUIRE(pid >= 0);
+
+       if (pid == 0) {
+               (void)close(rfd);
+               sleep(5);
+               (void)close(wfd);
+               _exit(0);
+       }
+       (void)close(wfd);
+
+       ATF_REQUIRE(clock_gettime(CLOCK_MONOTONIC, &ts1) == 0);
+       ATF_REQUIRE(poll(&pfd, 1, INFTIM) == 1);
+       ATF_REQUIRE(clock_gettime(CLOCK_MONOTONIC, &ts2) == 0);
+
+       /* Make sure at least a couple of seconds have elapsed. */
+       ATF_REQUIRE(ts2.tv_sec - ts1.tv_sec >= 2);
+



Home | Main Index | Thread Index | Old Index