Change Details

Currently, most protocols transition to a listening socket with something like the following: ``` SOCK_LOCK(so); error = solisten_proto_check(so); if (error) { SOCK_UNLOCK(so); return (error); } solisten_proto(so); SOCK_UNLOCK(so); ``` solisten_proto_check() fails if the socket is connected or connecting. However, the socket lock is not used to initiate I/O, so this pattern is racy. The change modifies solisten_proto_check() to additionally acquire socket buffer locks, and the calling thread holds them until solisten_proto() or solisten_proto_abort() is called. Now that the socket buffer locks are preserved by listen(2), this change allows socket I/O paths to properly interlock with listen(2). For an as-yet-unfixed example of what I'm talking about, look at soo_aio_queue(): it blindly assumes that it can safely lock a sockbuf and queue asynchronous I/O without checking for a listening socket. Without these changes, it is impossible to perform that check correctly without also acquiring the socket lock. The change makes listen(2) rather heavyweight, but I think this is the right tradeoff: listen(2) is called comparatively rarely, and we don't want to penalize I/O performance for rare cases. The SCTP case is still somewhat broken, since sctp_listen() may drop the inpcb lock to bind the socket. I believe this can be fixed by having a variant of sctp_inpcb_bind() which handles a locked inpcb, and by making sctp_listen() call SCTP_INP_INFO_WLOCK() before locking the PCB. I tried to mitigate it somewhat though.

Currently, most protocols transition to a listening socket with something like the following: ``` SOCK_LOCK(so); error = solisten_proto_check(so); if (error) { SOCK_UNLOCK(so); return (error); } solisten_proto(so); SOCK_UNLOCK(so); ``` solisten_proto_check() fails if the socket is connected or connecting. However, the socket lock is not used to initiate I/O, so this pattern is racy. The change modifies solisten_proto_check() to additionally acquire socket buffer locks, and the calling thread holds them until solisten_proto() or solisten_proto_abort() is called. Now that the socket buffer locks are preserved by listen(2), this change allows socket I/O paths to properly interlock with listen(2). For an as-yet-unfixed example of what I'm talking about, look at soo_aio_queue(): it blindly assumes that it can safely lock a sockbuf and queue asynchronous I/O without checking for a listening socket. Without these changes, it is impossible to perform that check correctly without also acquiring the socket lock. The change makes listen(2) rather heavyweight, but I think this is the right tradeoff: listen(2) is called comparatively rarely, and we don't want to penalize I/O performance for rare cases.

Currently, most protocols transition to a listening socket with something like the following: ``` SOCK_LOCK(so); error = solisten_proto_check(so); if (error) { SOCK_UNLOCK(so); return (error); } solisten_proto(so); SOCK_UNLOCK(so); ``` solisten_proto_check() fails if the socket is connected or connecting. However, the socket lock is not used to initiate I/O, so this pattern is racy. The change modifies solisten_proto_check() to additionally acquire socket buffer locks, and the calling thread holds them until solisten_proto() or solisten_proto_abort() is called. Now that the socket buffer locks are preserved by listen(2), this change allows socket I/O paths to properly interlock with listen(2). For an as-yet-unfixed example of what I'm talking about, look at soo_aio_queue(): it blindly assumes that it can safely lock a sockbuf and queue asynchronous I/O without checking for a listening socket. Without these changes, it is impossible to perform that check correctly without also acquiring the socket lock. The change makes listen(2) rather heavyweight, but I think this is the right tradeoff: listen(2) is called comparatively rarely, and we don't want to penalize I/O performance for rare cases. The SCTP case is still somewhat broken, since sctp_listen() may drop the inpcb lock to bind the socket. I believe this can be fixed by having a variant of sctp_inpcb_bind() which handles a locked inpcb, and by making sctp_listen() call SCTP_INP_INFO_WLOCK() before locking the PCB. I tried to mitigate it somewhat though.