[FreeBSD6.1-RELEASE]problem about soisconnected() in uipc_socket2.c

[Robert Watson]

On Tue, 13 Jun 2006, Blue wrote:

> I have found a questionable code in soisconnected() function in 
> uipc_socket2.c. In the very beginning, the SOCK_LOCK() would lock the 
> socket. However, in line 127, it unlocks socket. I am wondering that 
> SOCK_UNLOCK() should be called until all the socket's parameters are done. 
> In my opinion, it should be located right before ACCEPT_UNLOCK().

Some fields of the socket are protected by accept_mtx rather than the socket 
lock.  You can check the field locking key in socketvar.h for details:

/*-
  * Locking key to struct socket:
  * (a) constant after allocation, no locking required.
  * (b) locked by SOCK_LOCK(so).
  * (c) locked by SOCKBUF_LOCK(&so->so_rcv).
  * (d) locked by SOCKBUF_LOCK(&so->so_snd).
  * (e) locked by ACCEPT_LOCK().
  * (f) not locked since integer reads/writes are atomic.
  * (g) used only as a sleep/wakeup address, no value.
  * (h) locked by global mutex so_global_mtx.
  */

...
         int     so_qstate;              /* (e) internal state flags SQ_* */
         TAILQ_HEAD(, socket) so_incomp; /* (e) queue of partial unaccepted 
connections */
         TAILQ_HEAD(, socket) so_comp;   /* (e) queue of complete unaccepted 
connections */
         TAILQ_ENTRY(socket) so_list;    /* (e) list of unaccepted connections 
*/
         u_short so_qlen;                /* (e) number of unaccepted 
connections
*/
         u_short so_incqlen;             /* (e) number of unaccepted incomplete
                                            connections */
         u_short so_qlimit;              /* (e) max number queued connections 
*/

The socket lock is dropped at the earliest point in each case after which no 
further manipulation of the socket lock protected fields take place.  The only 
really dubious thing about what's there right now is the handling of socket 
upcalls, which has poorly defined locking properties, but I think in the 
non-accept filter case, the locking looks generally correct.  Right now we 
call so_upcall in different places with different locks, and some upcalls 
perform activities that may require acquiring further locks, possibly in bad 
orders.  We rely to some extent on the behavior of the protocol to keep 
certain races from happening, and that reliance is not well documented.  I 
have several works in progress related to socket locking which improve this to 
some extent.  One breaks out the single upcall function pointer into a series 
of upcalls each with well defined behavior; another coalesces the various 
socket locks, also resulting in well defined behavior.

Robert N M Watson
Computer Laboratory
Universty of Cambridge