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 +++++---------- 2 files changed, 28 insertions(+), 28 deletions(-) (limited to 'src/supplemental/http') 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 -- cgit v1.2.3-70-g09d2