[src/trunk]: src/sys If a multithreaded app closes an fd while another thread...

[dsl <dsl%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/13de3e8ed988
branches:  trunk
changeset: 750206:13de3e8ed988
user:      dsl <dsl%NetBSD.org@localhost>
date:      Sun Dec 20 09:36:05 2009 +0000

description:
If a multithreaded app closes an fd while another thread is blocked in
read/write/accept, then the expectation is that the blocked thread will
exit and the close complete.
Since only one fd is affected, but many fd can refer to the same file,
the close code can only request the fs code unblock with ERESTART.
Fixed for pipes and sockets, ERESTART will only be generated after such
a close - so there should be no change for other programs.
Also rename fo_abort() to fo_restart() (this used to be fo_drain()).
Fixes PR/26567

diffstat:

 sys/arch/xen/xen/xenevt.c        |   6 ++--
 sys/compat/svr4/svr4_net.c       |   6 ++--
 sys/compat/svr4_32/svr4_32_net.c |   6 ++--
 sys/dev/dmover/dmover_io.c       |   6 ++--
 sys/dev/putter/putter.c          |   6 ++--
 sys/kern/kern_descrip.c          |  22 +++++++++++++++-----
 sys/kern/kern_drvctl.c           |   6 ++--
 sys/kern/kern_event.c            |   6 ++--
 sys/kern/sys_mqueue.c            |   6 ++--
 sys/kern/sys_pipe.c              |  37 +++++++++++++++++++----------------
 sys/kern/sys_socket.c            |  10 ++++----
 sys/kern/uipc_socket.c           |  42 +++++++++++++++++++++++++++++++--------
 sys/kern/uipc_syscalls.c         |  10 +++++++-
 sys/kern/vfs_vnops.c             |   6 ++--
 sys/net/bpf.c                    |   6 ++--
 sys/net/if_tap.c                 |   6 ++--
 sys/opencrypto/cryptodev.c       |   6 ++--
 sys/sys/file.h                   |   6 ++--
 sys/sys/pipe.h                   |   3 +-
 sys/sys/socketvar.h              |   7 +++--
 20 files changed, 127 insertions(+), 82 deletions(-)

diffs (truncated from 823 to 300 lines):

diff -r 17a7b3cfd977 -r 13de3e8ed988 sys/arch/xen/xen/xenevt.c
--- a/sys/arch/xen/xen/xenevt.c Sun Dec 20 05:53:34 2009 +0000
+++ b/sys/arch/xen/xen/xenevt.c Sun Dec 20 09:36:05 2009 +0000
@@ -1,4 +1,4 @@
-/*      $NetBSD: xenevt.c,v 1.35 2009/12/09 21:32:58 dsl Exp $      */
+/*      $NetBSD: xenevt.c,v 1.36 2009/12/20 09:36:05 dsl Exp $      */
 
 /*
  * Copyright (c) 2005 Manuel Bouyer.
@@ -26,7 +26,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: xenevt.c,v 1.35 2009/12/09 21:32:58 dsl Exp $");
+__KERNEL_RCSID(0, "$NetBSD: xenevt.c,v 1.36 2009/12/20 09:36:05 dsl Exp $");
 
 #include "opt_xen.h"
 #include <sys/param.h>
@@ -81,7 +81,7 @@
        .fo_stat = fbadop_stat,
        .fo_close = xenevt_fclose,
        .fo_kqfilter = /* xenevt_fkqfilter */ fnullop_kqfilter,
-       .fo_abort = fnullop_abort,
+       .fo_restart = fnullop_restart,
 };
 
 dev_type_open(xenevtopen);
diff -r 17a7b3cfd977 -r 13de3e8ed988 sys/compat/svr4/svr4_net.c
--- a/sys/compat/svr4/svr4_net.c        Sun Dec 20 05:53:34 2009 +0000
+++ b/sys/compat/svr4/svr4_net.c        Sun Dec 20 09:36:05 2009 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: svr4_net.c,v 1.57 2009/12/09 21:32:58 dsl Exp $        */
+/*     $NetBSD: svr4_net.c,v 1.58 2009/12/20 09:36:05 dsl Exp $        */
 
 /*-
  * Copyright (c) 1994, 2008, 2009 The NetBSD Foundation, Inc.
@@ -34,7 +34,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: svr4_net.c,v 1.57 2009/12/09 21:32:58 dsl Exp $");
+__KERNEL_RCSID(0, "$NetBSD: svr4_net.c,v 1.58 2009/12/20 09:36:05 dsl Exp $");
 
 #define COMPAT_SVR4 1
 
@@ -108,7 +108,7 @@
        .fo_stat = soo_stat,
        .fo_close = svr4_soo_close,
        .fo_kqfilter = soo_kqfilter,
-       .fo_abort = soo_abort,
+       .fo_restart = soo_restart,
 };
 
 
diff -r 17a7b3cfd977 -r 13de3e8ed988 sys/compat/svr4_32/svr4_32_net.c
--- a/sys/compat/svr4_32/svr4_32_net.c  Sun Dec 20 05:53:34 2009 +0000
+++ b/sys/compat/svr4_32/svr4_32_net.c  Sun Dec 20 09:36:05 2009 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: svr4_32_net.c,v 1.20 2009/12/09 21:32:58 dsl Exp $      */
+/*     $NetBSD: svr4_32_net.c,v 1.21 2009/12/20 09:36:05 dsl Exp $      */
 
 /*-
  * Copyright (c) 1994, 2008, 2009 The NetBSD Foundation, Inc.
@@ -34,7 +34,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: svr4_32_net.c,v 1.20 2009/12/09 21:32:58 dsl Exp $");
+__KERNEL_RCSID(0, "$NetBSD: svr4_32_net.c,v 1.21 2009/12/20 09:36:05 dsl Exp $");
 
 #define COMPAT_SVR4 1
 
@@ -102,7 +102,7 @@
        .fo_poll = soo_poll,
        .fo_stat = soo_stat,
        .fo_close = svr4_soo_close,
-       .fo_abort = soo_abort,
+       .fo_restart = soo_restart,
 };
 
 
diff -r 17a7b3cfd977 -r 13de3e8ed988 sys/dev/dmover/dmover_io.c
--- a/sys/dev/dmover/dmover_io.c        Sun Dec 20 05:53:34 2009 +0000
+++ b/sys/dev/dmover/dmover_io.c        Sun Dec 20 09:36:05 2009 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: dmover_io.c,v 1.36 2009/12/09 21:32:58 dsl Exp $       */
+/*     $NetBSD: dmover_io.c,v 1.37 2009/12/20 09:36:05 dsl Exp $       */
 
 /*
  * Copyright (c) 2002, 2003 Wasabi Systems, Inc.
@@ -55,7 +55,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: dmover_io.c,v 1.36 2009/12/09 21:32:58 dsl Exp $");
+__KERNEL_RCSID(0, "$NetBSD: dmover_io.c,v 1.37 2009/12/20 09:36:05 dsl Exp $");
 
 #include <sys/param.h>
 #include <sys/queue.h>
@@ -761,7 +761,7 @@
        .fo_stat = dmio_stat,
        .fo_close = dmio_close,
        .fo_kqfilter = fnullop_kqfilter,
-       .fo_abort = fnullop_abort,
+       .fo_restart = fnullop_restart,
 };
 
 /*
diff -r 17a7b3cfd977 -r 13de3e8ed988 sys/dev/putter/putter.c
--- a/sys/dev/putter/putter.c   Sun Dec 20 05:53:34 2009 +0000
+++ b/sys/dev/putter/putter.c   Sun Dec 20 09:36:05 2009 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: putter.c,v 1.25 2009/12/09 21:32:59 dsl Exp $  */
+/*     $NetBSD: putter.c,v 1.26 2009/12/20 09:36:05 dsl Exp $  */
 
 /*
  * Copyright (c) 2006, 2007  Antti Kantee.  All Rights Reserved.
@@ -35,7 +35,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: putter.c,v 1.25 2009/12/09 21:32:59 dsl Exp $");
+__KERNEL_RCSID(0, "$NetBSD: putter.c,v 1.26 2009/12/20 09:36:05 dsl Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -206,7 +206,7 @@
        .fo_stat = putter_fop_stat,
        .fo_close = putter_fop_close,
        .fo_kqfilter = putter_fop_kqfilter,
-       .fo_abort = fnullop_abort,
+       .fo_restart = fnullop_restart,
 };
 
 static int
diff -r 17a7b3cfd977 -r 13de3e8ed988 sys/kern/kern_descrip.c
--- a/sys/kern/kern_descrip.c   Sun Dec 20 05:53:34 2009 +0000
+++ b/sys/kern/kern_descrip.c   Sun Dec 20 09:36:05 2009 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: kern_descrip.c,v 1.201 2009/12/09 21:32:59 dsl Exp $   */
+/*     $NetBSD: kern_descrip.c,v 1.202 2009/12/20 09:36:05 dsl Exp $   */
 
 /*-
  * Copyright (c) 2008, 2009 The NetBSD Foundation, Inc.
@@ -70,7 +70,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: kern_descrip.c,v 1.201 2009/12/09 21:32:59 dsl Exp $");
+__KERNEL_RCSID(0, "$NetBSD: kern_descrip.c,v 1.202 2009/12/20 09:36:05 dsl Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -610,7 +610,8 @@
                 * Wait for other references to drain.  This is typically
                 * an application error - the descriptor is being closed
                 * while still in use.
-                *
+                * (Or just a threaded application trying to unblock its
+                * thread that sleeps in (say) accept()).
                 */
                atomic_or_uint(&ff->ff_refcnt, FR_CLOSING);
 
@@ -623,8 +624,15 @@
                        knote_fdclose(fd);
                }
 
-               /* Try to drain out descriptor references. */
-               (*fp->f_ops->fo_abort)(fp);
+               /*
+                * Since the file system code doesn't know which fd
+                * each request came from (think dup()), we have to
+                * ask it to return ERESTART for any long-term blocks.
+                * The re-entry through read/write/etc will detect the
+                * closed fd and return EBAFD.
+                * Blocked partial writes may return a short length.
+                */
+               (*fp->f_ops->fo_restart)(fp);
                mutex_enter(&fdp->fd_lock);
 
                /*
@@ -632,6 +640,8 @@
                 * in order to ensure that all pre-existing references
                 * have been drained.  New references past this point are
                 * of no interest.
+                * XXX (dsl) this may need to call fo_restart() after a
+                * timeout to guarantee that all the system calls exit.
                 */
                while ((ff->ff_refcnt & FR_MASK) != 0) {
                        cv_wait(&ff->ff_closing, &fdp->fd_lock);
@@ -1787,7 +1797,7 @@
 }
 
 void
-fnullop_abort(file_t *fp)
+fnullop_restart(file_t *fp)
 {
 
 }
diff -r 17a7b3cfd977 -r 13de3e8ed988 sys/kern/kern_drvctl.c
--- a/sys/kern/kern_drvctl.c    Sun Dec 20 05:53:34 2009 +0000
+++ b/sys/kern/kern_drvctl.c    Sun Dec 20 09:36:05 2009 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: kern_drvctl.c,v 1.30 2009/12/09 21:32:59 dsl Exp $ */
+/* $NetBSD: kern_drvctl.c,v 1.31 2009/12/20 09:36:05 dsl Exp $ */
 
 /*
  * Copyright (c) 2004
@@ -27,7 +27,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: kern_drvctl.c,v 1.30 2009/12/09 21:32:59 dsl Exp $");
+__KERNEL_RCSID(0, "$NetBSD: kern_drvctl.c,v 1.31 2009/12/20 09:36:05 dsl Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -90,7 +90,7 @@
        .fo_stat = drvctl_stat,
        .fo_close = drvctl_close,
        .fo_kqfilter = fnullop_kqfilter,
-       .fo_abort = fnullop_abort,
+       .fo_restart = fnullop_restart,
 };
 
 #define MAXLOCATORS 100
diff -r 17a7b3cfd977 -r 13de3e8ed988 sys/kern/kern_event.c
--- a/sys/kern/kern_event.c     Sun Dec 20 05:53:34 2009 +0000
+++ b/sys/kern/kern_event.c     Sun Dec 20 09:36:05 2009 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: kern_event.c,v 1.67 2009/12/09 21:32:59 dsl Exp $      */
+/*     $NetBSD: kern_event.c,v 1.68 2009/12/20 09:36:05 dsl Exp $      */
 
 /*-
  * Copyright (c) 2008, 2009 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: kern_event.c,v 1.67 2009/12/09 21:32:59 dsl Exp $");
+__KERNEL_RCSID(0, "$NetBSD: kern_event.c,v 1.68 2009/12/20 09:36:05 dsl Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -115,7 +115,7 @@
        .fo_stat = kqueue_stat,
        .fo_close = kqueue_close,
        .fo_kqfilter = kqueue_kqfilter,
-       .fo_abort = fnullop_abort,
+       .fo_restart = fnullop_restart,
 };
 
 static const struct filterops kqread_filtops =
diff -r 17a7b3cfd977 -r 13de3e8ed988 sys/kern/sys_mqueue.c
--- a/sys/kern/sys_mqueue.c     Sun Dec 20 05:53:34 2009 +0000
+++ b/sys/kern/sys_mqueue.c     Sun Dec 20 09:36:05 2009 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: sys_mqueue.c,v 1.28 2009/12/10 12:22:48 drochner Exp $ */
+/*     $NetBSD: sys_mqueue.c,v 1.29 2009/12/20 09:36:05 dsl Exp $      */
 
 /*
  * Copyright (c) 2007-2009 Mindaugas Rasiukevicius <rmind at NetBSD org>
@@ -42,7 +42,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: sys_mqueue.c,v 1.28 2009/12/10 12:22:48 drochner Exp $");
+__KERNEL_RCSID(0, "$NetBSD: sys_mqueue.c,v 1.29 2009/12/20 09:36:05 dsl Exp $");
 
 #include <sys/param.h>
 #include <sys/types.h>
@@ -103,7 +103,7 @@
        .fo_stat = mq_stat_fop,
        .fo_close = mq_close_fop,
        .fo_kqfilter = fnullop_kqfilter,
-       .fo_abort = fnullop_abort,
+       .fo_restart = fnullop_restart,
 };
 
 static const struct syscall_package mqueue_syscalls[] = {
diff -r 17a7b3cfd977 -r 13de3e8ed988 sys/kern/sys_pipe.c
--- a/sys/kern/sys_pipe.c       Sun Dec 20 05:53:34 2009 +0000
+++ b/sys/kern/sys_pipe.c       Sun Dec 20 09:36:05 2009 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: sys_pipe.c,v 1.126 2009/12/15 18:35:18 dsl Exp $       */
+/*     $NetBSD: sys_pipe.c,v 1.127 2009/12/20 09:36:06 dsl Exp $       */
 
 /*-
  * Copyright (c) 2003, 2007, 2008, 2009 The NetBSD Foundation, Inc.
@@ -68,7 +68,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: sys_pipe.c,v 1.126 2009/12/15 18:35:18 dsl Exp $");
+__KERNEL_RCSID(0, "$NetBSD: sys_pipe.c,v 1.127 2009/12/20 09:36:06 dsl Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -109,7 +109,7 @@
 static int     pipe_kqfilter(file_t *, struct knote *);