d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

In the current world order, each socket has two mutexes: a mutex

Browse Source 
that protects socket and receive socket buffer state, and a second
mutex to protect send socket buffer state.  In some places, the
mutex shared between the socket and receive socket buffer will be
acquired twice, once by each layer, resulting in some
inconsistency, but providing the abstraction benefit of being able
to more easily separate the two mutexes in the future if desired.

When transitioning a socket to the SS_ISDISCONNECTING or
SS_ISDISCONNECTED states, grab the socket/receive socket buffer lock
once rather than grabbing it as the socket lock, modifying socket
state, then grabbing a second time as the receive lock in order to
modify the socket buffer state to indicate no further data can be
read.  This change is believed to close a race between the change in
socket state and the change in socket buffer state, which for a
remotely initiated close on a UNIX domain socket, resulted in
soreceive() returning ENOTCONN rather than an EOF condition.

A similar race still exists in the case of send, however, and is
harder to fix as the socket and send socket buffer mutexes are not
the same, and we would like to avoid holding combinations of socket
mutexes over sb_upcall until we've finished clarifying the locking
protocol for upcalls.

This change has the side affect of reducing the number of mutex
operations to initiate disconnect or perform disconnect on a
socket by two.

PR:		78824
Rerported by:	Marc Olzheim <marcolz@stack.nl>
MFC after:	2 weeks
This commit is contained in:
Robert Watson 2005-05-27 17:16:43 +00:00
parent 0cb7166f78
commit 83b3d58d05
Notes: svn2git 2020-12-20 02:59:44 +00:00 
svn path=/head/; revision=146688
2 changed files with 12 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
sys/kern/uipc_sockbuf.c
View File

@ -159,15 +159,12 @@ soisdisconnecting(so)
{
/*
* XXXRW: This code separately acquires SOCK_LOCK(so) and
* SOCKBUF_LOCK(&so->so_rcv) even though they are the same mutex to
* avoid introducing the assumption that they are the same.
* XXXRW: This code assumes that SOCK_LOCK(so) and
* SOCKBUF_LOCK(&so->so_rcv) are the same.
*/
SOCK_LOCK(so);
SOCKBUF_LOCK(&so->so_rcv);
so->so_state &= ~SS_ISCONNECTING;
so->so_state |= SS_ISDISCONNECTING;
SOCK_UNLOCK(so);
SOCKBUF_LOCK(&so->so_rcv);
so->so_rcv.sb_state |= SBS_CANTRCVMORE;
sorwakeup_locked(so);
SOCKBUF_LOCK(&so->so_snd);
@ -182,16 +179,12 @@ soisdisconnected(so)
{
/*
* XXXRW: This code separately acquires SOCK_LOCK(so) and
* SOCKBUF_LOCK(&so->so_rcv) even though they are the same mutex to
* avoid introducing the assumption that they are the same.
* XXXRW: This code assumes that SOCK_LOCK(so) and
* SOCKBUF_LOCK(&so->so_rcv) are the same.
*/
/* XXXRW: so_state locking? */
SOCK_LOCK(so);
SOCKBUF_LOCK(&so->so_rcv);
so->so_state &= ~(SS_ISCONNECTING|SS_ISCONNECTED|SS_ISDISCONNECTING);
so->so_state |= SS_ISDISCONNECTED;
SOCK_UNLOCK(so);
SOCKBUF_LOCK(&so->so_rcv);
so->so_rcv.sb_state |= SBS_CANTRCVMORE;
sorwakeup_locked(so);
SOCKBUF_LOCK(&so->so_snd);

19
sys/kern/uipc_socket2.c
View File

@ -159,15 +159,12 @@ soisdisconnecting(so)
{
/*
* XXXRW: This code separately acquires SOCK_LOCK(so) and
* SOCKBUF_LOCK(&so->so_rcv) even though they are the same mutex to
* avoid introducing the assumption that they are the same.
* XXXRW: This code assumes that SOCK_LOCK(so) and
* SOCKBUF_LOCK(&so->so_rcv) are the same.
*/
SOCK_LOCK(so);
SOCKBUF_LOCK(&so->so_rcv);
so->so_state &= ~SS_ISCONNECTING;
so->so_state |= SS_ISDISCONNECTING;
SOCK_UNLOCK(so);
SOCKBUF_LOCK(&so->so_rcv);
so->so_rcv.sb_state |= SBS_CANTRCVMORE;
sorwakeup_locked(so);
SOCKBUF_LOCK(&so->so_snd);
@ -182,16 +179,12 @@ soisdisconnected(so)
{
/*
* XXXRW: This code separately acquires SOCK_LOCK(so) and
* SOCKBUF_LOCK(&so->so_rcv) even though they are the same mutex to
* avoid introducing the assumption that they are the same.
* XXXRW: This code assumes that SOCK_LOCK(so) and
* SOCKBUF_LOCK(&so->so_rcv) are the same.
*/
/* XXXRW: so_state locking? */
SOCK_LOCK(so);
SOCKBUF_LOCK(&so->so_rcv);
so->so_state &= ~(SS_ISCONNECTING|SS_ISCONNECTED|SS_ISDISCONNECTING);
so->so_state |= SS_ISDISCONNECTED;
SOCK_UNLOCK(so);
SOCKBUF_LOCK(&so->so_rcv);
so->so_rcv.sb_state |= SBS_CANTRCVMORE;
sorwakeup_locked(so);
SOCKBUF_LOCK(&so->so_snd);