From owner-freebsd-hackers@FreeBSD.ORG Fri Apr 5 20:32:25 2013 Return-Path: Delivered-To: freebsd-hackers@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 4A8399FC; Fri, 5 Apr 2013 20:32:25 +0000 (UTC) (envelope-from jilles@stack.nl) Received: from mx1.stack.nl (relay02.stack.nl [IPv6:2001:610:1108:5010::104]) by mx1.freebsd.org (Postfix) with ESMTP id 12FD8C14; Fri, 5 Apr 2013 20:32:25 +0000 (UTC) Received: from snail.stack.nl (snail.stack.nl [IPv6:2001:610:1108:5010::131]) by mx1.stack.nl (Postfix) with ESMTP id 0D1703592EE; Fri, 5 Apr 2013 22:32:23 +0200 (CEST) Received: by snail.stack.nl (Postfix, from userid 1677) id DF9DE2848C; Fri, 5 Apr 2013 22:32:22 +0200 (CEST) Date: Fri, 5 Apr 2013 22:32:22 +0200 From: Jilles Tjoelker To: Andriy Gapon Subject: Re: close(2) while accept(2) is blocked Message-ID: <20130405203222.GB10840@stack.nl> References: <515475C7.6010404@FreeBSD.org> <201304011122.13101.jhb@freebsd.org> <515DBBB5.70505@FreeBSD.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <515DBBB5.70505@FreeBSD.org> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: freebsd-net@FreeBSD.org, office@FreeBSD.org, FreeBSD Hackers X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 05 Apr 2013 20:32:25 -0000 On Thu, Apr 04, 2013 at 08:43:17PM +0300, Andriy Gapon wrote: > on 01/04/2013 18:22 John Baldwin said the following: > > I think you need to split the 'struct file' reference count into two > > different counts similar to the how we have vref/vrele vs > > vhold/vdrop for vnodes. The fget for accept and probably most other > > system calls should probably be equivalent to vhold, whereas things > > like open/dup (and storing an fd in a cmsg) should be more like > > vref. close() should then be a vrele(). > This model makes perfect sense. > Unfortunately, ENOTIME to work on this. This looks like it can work but I don't know whether it's worth the trouble. > Meanwhile I am using the following patch specific to local domain > sockets, accept(2) and shutdown(2). Turns out that the problematic > application does both shutdown(RDWR) and close(2), but that doesn't > help on FreeBSD. > BTW, this is the application: > http://thread.gmane.org/gmane.os.freebsd.devel.office/1754 > The patch does help. > Author: Andriy Gapon > Date: Thu Mar 28 20:08:13 2013 +0200 > > [test!] uipc_shutdown: use soisdisconnected instead of socantsendmore > > So that in addition to disabling sends we also wake up threads blocked > in accept (on so_timeo in general). > > diff --git a/sys/kern/uipc_usrreq.c b/sys/kern/uipc_usrreq.c > index 9b60eab..e93d46c 100644 > --- a/sys/kern/uipc_usrreq.c > +++ b/sys/kern/uipc_usrreq.c > @@ -1074,7 +1074,7 @@ uipc_shutdown(struct socket *so) > > UNP_LINK_WLOCK(); > UNP_PCB_LOCK(unp); > - socantsendmore(so); > + soisdisconnected(so); > unp_shutdown(unp); > UNP_PCB_UNLOCK(unp); > UNP_LINK_WUNLOCK(); I think this patch makes shutdown(SHUT_WR) on unix domain sockets act like shutdown(SHUT_RDWR), i.e. receives are incorrectly denied. The below patch also makes shutdown(SHUT_RDWR) abort a blocking accept on a unix domain socket, but it should work for all domains: Index: sys/kern/uipc_socket.c =================================================================== --- sys/kern/uipc_socket.c (revision 248873) +++ sys/kern/uipc_socket.c (working copy) @@ -2428,9 +2428,11 @@ soshutdown(struct socket *so, int how) sorflush(so); if (how != SHUT_RD) { error = (*pr->pr_usrreqs->pru_shutdown)(so); + wakeup(&so->so_timeo); CURVNET_RESTORE(); return (error); } + wakeup(&so->so_timeo); CURVNET_RESTORE(); return (0); } A blocking accept (and some other operations) is waiting on &so->so_timeo. Once it wakes up, it will detect the SBS_CANTRCVMORE bit. A spurious wakeup on so->so_timeo appears harmless (sleep retried) except when lingering on close (SO_LINGER) so this should be fairly safe. -- Jilles Tjoelker