From 1d033484ee1a2ec26d3eead073e7bc0f889ffdf4 Mon Sep 17 00:00:00 2001 From: Garrett D'Amore Date: Tue, 15 May 2018 01:47:12 -0700 Subject: fixes #419 want to nni_aio_stop without blocking (#428) * fixes #419 want to nni_aio_stop without blocking This actually introduces an nni_aio_close() API that causes nni_aio_begin to return NNG_ECLOSED, while scheduling a callback on the AIO to do an NNG_ECLOSED as well. This should be called in non-blocking close() contexts instead of nni_aio_stop(), and the cases where we call nni_aio_fini() multiple times are updated updated to add nni_aio_stop() calls on all "interlinked" aios before finalizing them. Furthermore, we call nni_aio_close() as soon as practical in the close path. This closes an annoying race condition where the callback from a lower subsystem could wind up rescheduling an operation that we wanted to abort. --- src/protocol/reqrep0/rep.c | 16 +++++++++++++--- src/protocol/reqrep0/req.c | 19 +++++++++++++------ src/protocol/reqrep0/xrep.c | 25 ++++++++++++++++++------- src/protocol/reqrep0/xreq.c | 25 +++++++++++++++++-------- 4 files changed, 61 insertions(+), 24 deletions(-) (limited to 'src/protocol/reqrep0') diff --git a/src/protocol/reqrep0/rep.c b/src/protocol/reqrep0/rep.c index 965cbea7..c483b777 100644 --- a/src/protocol/reqrep0/rep.c +++ b/src/protocol/reqrep0/rep.c @@ -296,6 +296,15 @@ rep0_sock_close(void *arg) rep0_ctx_close(s->ctx); } +static void +rep0_pipe_stop(void *arg) +{ + rep0_pipe *p = arg; + + nni_aio_stop(p->aio_send); + nni_aio_stop(p->aio_recv); +} + static void rep0_pipe_fini(void *arg) { @@ -347,14 +356,14 @@ rep0_pipe_start(void *arg) } static void -rep0_pipe_stop(void *arg) +rep0_pipe_close(void *arg) { rep0_pipe *p = arg; rep0_sock *s = p->rep; rep0_ctx * ctx; - nni_aio_stop(p->aio_send); - nni_aio_stop(p->aio_recv); + nni_aio_close(p->aio_send); + nni_aio_close(p->aio_recv); nni_mtx_lock(&s->lk); if (nni_list_active(&s->recvpipes, p)) { @@ -647,6 +656,7 @@ static nni_proto_pipe_ops rep0_pipe_ops = { .pipe_init = rep0_pipe_init, .pipe_fini = rep0_pipe_fini, .pipe_start = rep0_pipe_start, + .pipe_close = rep0_pipe_close, .pipe_stop = rep0_pipe_stop, }; diff --git a/src/protocol/reqrep0/req.c b/src/protocol/reqrep0/req.c index bbb0b886..fc9d7271 100644 --- a/src/protocol/reqrep0/req.c +++ b/src/protocol/reqrep0/req.c @@ -185,6 +185,15 @@ req0_sock_fini(void *arg) NNI_FREE_STRUCT(s); } +static void +req0_pipe_stop(void *arg) +{ + req0_pipe *p = arg; + + nni_aio_stop(p->aio_recv); + nni_aio_stop(p->aio_send); +} + static void req0_pipe_fini(void *arg) { @@ -243,17 +252,14 @@ req0_pipe_start(void *arg) } static void -req0_pipe_stop(void *arg) +req0_pipe_close(void *arg) { req0_pipe *p = arg; req0_sock *s = p->req; req0_ctx * ctx; - nni_aio_stop(p->aio_recv); - nni_aio_stop(p->aio_send); - - // At this point there should not be any further AIOs running. - // Further, any completion tasks have completed. + nni_aio_close(p->aio_recv); + nni_aio_close(p->aio_send); nni_mtx_lock(&s->mtx); // This removes the node from either busypipes or readypipes. @@ -837,6 +843,7 @@ static nni_proto_pipe_ops req0_pipe_ops = { .pipe_init = req0_pipe_init, .pipe_fini = req0_pipe_fini, .pipe_start = req0_pipe_start, + .pipe_close = req0_pipe_close, .pipe_stop = req0_pipe_stop, }; diff --git a/src/protocol/reqrep0/xrep.c b/src/protocol/reqrep0/xrep.c index 4773677e..6dcfe6be 100644 --- a/src/protocol/reqrep0/xrep.c +++ b/src/protocol/reqrep0/xrep.c @@ -62,7 +62,6 @@ xrep0_sock_fini(void *arg) { xrep0_sock *s = arg; - nni_aio_stop(s->aio_getq); nni_aio_fini(s->aio_getq); nni_idhash_fini(s->pipes); nni_mtx_fini(&s->lk); @@ -108,7 +107,18 @@ xrep0_sock_close(void *arg) { xrep0_sock *s = arg; - nni_aio_abort(s->aio_getq, NNG_ECLOSED); + nni_aio_close(s->aio_getq); +} + +static void +xrep0_pipe_stop(void *arg) +{ + xrep0_pipe *p = arg; + + nni_aio_stop(p->aio_getq); + nni_aio_stop(p->aio_send); + nni_aio_stop(p->aio_recv); + nni_aio_stop(p->aio_putq); } static void @@ -178,16 +188,16 @@ xrep0_pipe_start(void *arg) } static void -xrep0_pipe_stop(void *arg) +xrep0_pipe_close(void *arg) { xrep0_pipe *p = arg; xrep0_sock *s = p->rep; + nni_aio_close(p->aio_getq); + nni_aio_close(p->aio_send); + nni_aio_close(p->aio_recv); + nni_aio_close(p->aio_putq); nni_msgq_close(p->sendq); - nni_aio_stop(p->aio_getq); - nni_aio_stop(p->aio_send); - nni_aio_stop(p->aio_recv); - nni_aio_stop(p->aio_putq); nni_mtx_lock(&s->lk); nni_idhash_remove(s->pipes, nni_pipe_id(p->pipe)); @@ -389,6 +399,7 @@ static nni_proto_pipe_ops xrep0_pipe_ops = { .pipe_init = xrep0_pipe_init, .pipe_fini = xrep0_pipe_fini, .pipe_start = xrep0_pipe_start, + .pipe_close = xrep0_pipe_close, .pipe_stop = xrep0_pipe_stop, }; diff --git a/src/protocol/reqrep0/xreq.c b/src/protocol/reqrep0/xreq.c index 13ae7418..793411af 100644 --- a/src/protocol/reqrep0/xreq.c +++ b/src/protocol/reqrep0/xreq.c @@ -89,6 +89,17 @@ xreq0_sock_fini(void *arg) NNI_FREE_STRUCT(s); } +static void +xreq0_pipe_stop(void *arg) +{ + xreq0_pipe *p = arg; + + nni_aio_stop(p->aio_getq); + nni_aio_stop(p->aio_putq); + nni_aio_stop(p->aio_recv); + nni_aio_stop(p->aio_send); +} + static void xreq0_pipe_fini(void *arg) { @@ -140,17 +151,14 @@ xreq0_pipe_start(void *arg) } static void -xreq0_pipe_stop(void *arg) +xreq0_pipe_close(void *arg) { xreq0_pipe *p = arg; - nni_aio_stop(p->aio_getq); - nni_aio_stop(p->aio_putq); - nni_aio_stop(p->aio_recv); - nni_aio_stop(p->aio_send); - - // At this point there should not be any further AIOs running. - // Further, any completion tasks have completed. + nni_aio_close(p->aio_getq); + nni_aio_close(p->aio_putq); + nni_aio_close(p->aio_recv); + nni_aio_close(p->aio_send); } // For raw mode we can just let the pipes "contend" via getq to get a @@ -277,6 +285,7 @@ static nni_proto_pipe_ops xreq0_pipe_ops = { .pipe_init = xreq0_pipe_init, .pipe_fini = xreq0_pipe_fini, .pipe_start = xreq0_pipe_start, + .pipe_close = xreq0_pipe_close, .pipe_stop = xreq0_pipe_stop, }; -- cgit v1.2.3-70-g09d2