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/supplemental/http/http_conn.c | 40 +++++++++++++++++++--------------- src/supplemental/http/http_server.c | 16 +++++--------- src/supplemental/tls/mbedtls/tls.c | 3 +++ src/supplemental/websocket/websocket.c | 11 +++++----- 4 files changed, 37 insertions(+), 33 deletions(-) (limited to 'src/supplemental') diff --git a/src/supplemental/http/http_conn.c b/src/supplemental/http/http_conn.c index f3b16370..15d1f776 100644 --- a/src/supplemental/http/http_conn.c +++ b/src/supplemental/http/http_conn.c @@ -110,22 +110,26 @@ http_close(nni_http_conn *conn) } conn->closed = true; - if (nni_list_first(&conn->wrq)) { - nni_aio_abort(conn->wr_aio, NNG_ECLOSED); - // Abort all operations except the one in flight. - while ((aio = nni_list_last(&conn->wrq)) != - nni_list_first(&conn->wrq)) { - nni_aio_list_remove(aio); - nni_aio_finish_error(aio, NNG_ECLOSED); - } + nni_aio_close(conn->wr_aio); + nni_aio_close(conn->rd_aio); + + if ((aio = conn->rd_uaio) != NULL) { + conn->rd_uaio = NULL; + nni_aio_finish_error(aio, NNG_ECLOSED); } - if (nni_list_first(&conn->rdq)) { - nni_aio_abort(conn->rd_aio, NNG_ECLOSED); - while ((aio = nni_list_last(&conn->rdq)) != - nni_list_first(&conn->rdq)) { - nni_aio_list_remove(aio); - nni_aio_finish_error(aio, NNG_ECLOSED); - } + if ((aio = conn->wr_uaio) != NULL) { + conn->wr_uaio = NULL; + nni_aio_finish_error(aio, NNG_ECLOSED); + } + + // Abort all operations except the one in flight. + while ((aio = nni_list_first(&conn->wrq)) != NULL) { + nni_aio_list_remove(aio); + nni_aio_finish_error(aio, NNG_ECLOSED); + } + while ((aio = nni_list_first(&conn->rdq)) != NULL) { + nni_aio_list_remove(aio); + nni_aio_finish_error(aio, NNG_ECLOSED); } if (conn->sock != NULL) { @@ -668,6 +672,9 @@ nni_http_tls_verified(nni_http_conn *conn) void nni_http_conn_fini(nni_http_conn *conn) { + nni_aio_stop(conn->wr_aio); + nni_aio_stop(conn->rd_aio); + nni_mtx_lock(&conn->mtx); http_close(conn); if ((conn->sock != NULL) && (conn->fini != NULL)) { @@ -675,8 +682,7 @@ nni_http_conn_fini(nni_http_conn *conn) conn->sock = NULL; } nni_mtx_unlock(&conn->mtx); - nni_aio_stop(conn->wr_aio); - nni_aio_stop(conn->rd_aio); + nni_aio_fini(conn->wr_aio); nni_aio_fini(conn->rd_aio); nni_free(conn->rd_buf, conn->rd_bufsz); diff --git a/src/supplemental/http/http_server.c b/src/supplemental/http/http_server.c index c92de586..c7738aee 100644 --- a/src/supplemental/http/http_server.c +++ b/src/supplemental/http/http_server.c @@ -228,12 +228,6 @@ http_sconn_reap(void *arg) NNI_FREE_STRUCT(sc); } -static void -http_sconn_fini(http_sconn *sc) -{ - nni_reap(&sc->reap, http_sconn_reap, sc); -} - static void http_sconn_close_locked(http_sconn *sc) { @@ -245,15 +239,15 @@ http_sconn_close_locked(http_sconn *sc) NNI_ASSERT(!sc->finished); sc->closed = true; - nni_aio_abort(sc->rxaio, NNG_ECLOSED); - nni_aio_abort(sc->txaio, NNG_ECLOSED); - nni_aio_abort(sc->txdataio, NNG_ECLOSED); - nni_aio_abort(sc->cbaio, NNG_ECLOSED); + nni_aio_close(sc->rxaio); + nni_aio_close(sc->txaio); + nni_aio_close(sc->txdataio); + nni_aio_close(sc->cbaio); if ((conn = sc->conn) != NULL) { nni_http_conn_close(conn); } - http_sconn_fini(sc); + nni_reap(&sc->reap, http_sconn_reap, sc); } static void diff --git a/src/supplemental/tls/mbedtls/tls.c b/src/supplemental/tls/mbedtls/tls.c index f721d4aa..cdd226cd 100644 --- a/src/supplemental/tls/mbedtls/tls.c +++ b/src/supplemental/tls/mbedtls/tls.c @@ -761,6 +761,9 @@ nni_tls_close(nni_tls *tp) { nni_aio *aio; + nni_aio_close(tp->tcp_send); + nni_aio_close(tp->tcp_recv); + nni_mtx_lock(&tp->lk); tp->tls_closed = true; diff --git a/src/supplemental/websocket/websocket.c b/src/supplemental/websocket/websocket.c index 9a3f5519..6d4d3c13 100644 --- a/src/supplemental/websocket/websocket.c +++ b/src/supplemental/websocket/websocket.c @@ -407,14 +407,15 @@ ws_close_cb(void *arg) nni_ws *ws = arg; ws_msg *wm; + nni_aio_close(ws->txaio); + nni_aio_close(ws->rxaio); + nni_aio_close(ws->httpaio); + // Either we sent a close frame, or we didn't. Either way, // we are done, and its time to abort everything else. nni_mtx_lock(&ws->mtx); nni_http_conn_close(ws->http); - nni_aio_abort(ws->txaio, NNG_ECLOSED); - nni_aio_abort(ws->rxaio, NNG_ECLOSED); - nni_aio_abort(ws->httpaio, NNG_ECLOSED); // This list (receive) should be empty. while ((wm = nni_list_first(&ws->rxmsgs)) != NULL) { @@ -464,8 +465,8 @@ ws_close(nni_ws *ws, uint16_t code) // pending connect request. if (!ws->closed) { // ABORT connection negotiation. - nni_aio_abort(ws->connaio, NNG_ECLOSED); - nni_aio_abort(ws->httpaio, NNG_ECLOSED); + nni_aio_close(ws->connaio); + nni_aio_close(ws->httpaio); ws_send_close(ws, code); } -- cgit v1.2.3-70-g09d2