From ab7772be3e3c208a48408b67924d3b58fca7f474 Mon Sep 17 00:00:00 2001 From: Garrett D'Amore Date: Tue, 18 Jul 2017 11:00:08 -0700 Subject: Fix close-related leak of pipes. We have seen leaks of pipes causing test failures (e.g. the Windows IPC test) due to EADDRINUSE. This was caused by a case where we failed to pass the pipe up because the AIO had already been canceled, and we didn't realize that we had oprhaned the pipe. The fix is to add a return value to nni_aio_finish, and verify that we did finish properly, or if we did not then we must free the pipe ourself. (The zero return from nni_aio_finish indicates that it accepts ownership of resources passed via the aio.) --- src/transport/inproc/inproc.c | 7 ++++++- src/transport/ipc/ipc.c | 10 +++++++--- src/transport/tcp/tcp.c | 7 +++++-- 3 files changed, 18 insertions(+), 6 deletions(-) (limited to 'src/transport') diff --git a/src/transport/inproc/inproc.c b/src/transport/inproc/inproc.c index 68490747..226a31ce 100644 --- a/src/transport/inproc/inproc.c +++ b/src/transport/inproc/inproc.c @@ -257,7 +257,12 @@ nni_inproc_conn_finish(nni_aio *aio, int rv) } } } - nni_aio_finish(aio, rv, 0); + if (nni_aio_finish(aio, rv, 0) != 0) { + if (aio->a_pipe != NULL) { + nni_inproc_pipe_fini(aio->a_pipe); + aio->a_pipe = NULL; + } + } } static void diff --git a/src/transport/ipc/ipc.c b/src/transport/ipc/ipc.c index b799dba1..862cf955 100644 --- a/src/transport/ipc/ipc.c +++ b/src/transport/ipc/ipc.c @@ -551,7 +551,11 @@ nni_ipc_ep_finish(nni_ipc_ep *ep) done: ep->aio.a_pipe = NULL; - nni_aio_finish(aio, rv, 0); + if (nni_aio_finish(aio, rv, 0) != 0) { + if (rv == 0) { + nni_ipc_pipe_fini(pipe); + } + } } static void @@ -592,7 +596,6 @@ nni_ipc_ep_accept(void *arg, nni_aio *aio) } ep->user_aio = aio; - // If we can't start, then its dying and we can't report either, if ((rv = nni_aio_start(aio, nni_ipc_cancel_ep, ep)) != 0) { ep->user_aio = NULL; nni_mtx_unlock(&ep->mtx); @@ -620,7 +623,8 @@ nni_ipc_ep_connect(void *arg, nni_aio *aio) ep->user_aio = aio; - // If we can't start, then its dying and we can't report either, + // If we can't start, then its dying and we can't report + // either, if ((rv = nni_aio_start(aio, nni_ipc_cancel_ep, ep)) != 0) { ep->user_aio = NULL; nni_mtx_unlock(&ep->mtx); diff --git a/src/transport/tcp/tcp.c b/src/transport/tcp/tcp.c index dad4c46e..23472883 100644 --- a/src/transport/tcp/tcp.c +++ b/src/transport/tcp/tcp.c @@ -618,7 +618,11 @@ nni_tcp_ep_finish(nni_tcp_ep *ep) done: ep->aio.a_pipe = NULL; - nni_aio_finish(aio, rv, 0); + if (nni_aio_finish(aio, rv, 0) != 0) { + if (rv == 0) { + nni_tcp_pipe_fini(pipe); + } + } } static void @@ -657,7 +661,6 @@ nni_tcp_ep_accept(void *arg, nni_aio *aio) NNI_ASSERT(ep->user_aio == NULL); ep->user_aio = aio; - // If we can't start, then its dying and we can't report either, if ((rv = nni_aio_start(aio, nni_tcp_cancel_ep, ep)) != 0) { ep->user_aio = NULL; nni_mtx_unlock(&ep->mtx); -- cgit v1.2.3-70-g09d2