close(2) while accept(2) is blocked

[Jilles Tjoelker]

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 <avg at icyb.net.ua>
> 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