Skip to content

Commit

Permalink
Socket close may leak messages.
Browse files Browse the repository at this point in the history
We try to move the msgq close up earler.  While here we can stop
dropping and reacquiring the lock -- this is likely left over
and may lead to races.
  • Loading branch information
gdamore authored and JaylinYu committed Feb 8, 2023
1 parent 8fa752c commit d513c64
Showing 1 changed file with 5 additions and 8 deletions.
13 changes: 5 additions & 8 deletions src/core/socket.c
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ struct nni_ctx {
nni_list_node c_node;
nni_sock *c_sock;
nni_proto_ctx_ops c_ops;
void *c_data;
void *c_data;
size_t c_size;
bool c_closed;
unsigned c_ref; // protected by global lock
Expand Down Expand Up @@ -704,6 +704,10 @@ nni_sock_shutdown(nni_sock *sock)
}
nni_mtx_unlock(&sock->s_mx);

// Close the upper queues immediately.
nni_msgq_close(sock->s_urq);
nni_msgq_close(sock->s_uwq);

// We now mark any owned contexts as closing.
// XXX: Add context draining support here!
nni_mtx_lock(&sock_lk);
Expand All @@ -720,14 +724,12 @@ nni_sock_shutdown(nni_sock *sock)
// If still has a reference count, then wait for last
// reference to close before nuking it.
}
nni_mtx_unlock(&sock_lk);

// Generally, unless the protocol is blocked trying to perform
// writes (e.g. a slow reader on the other side), it should be
// trying to shut things down. We wait to give it
// a chance to do so gracefully.

nni_mtx_lock(&sock_lk);
while (!nni_list_empty(&sock->s_ctxs)) {
sock->s_ctxwait = true;
nni_cv_wait(&sock->s_close_cv);
Expand All @@ -740,11 +742,6 @@ nni_sock_shutdown(nni_sock *sock)
// give the protocol a chance to flush its write side. Now
// it is time to be a little more insistent.

// Close the upper queues immediately. This can happen
// safely while we hold the lock.
nni_msgq_close(sock->s_urq);
nni_msgq_close(sock->s_uwq);

// For each pipe, arrange for it to teardown hard. We would
// expect there not to be any here.
NNI_LIST_FOREACH (&sock->s_pipes, pipe) {
Expand Down

0 comments on commit d513c64

Please sign in to comment.