From 0f0cc86581c6d56a40d445de3adbd152af1dbaa5 Mon Sep 17 00:00:00 2001 From: Garrett D'Amore Date: Sun, 8 Dec 2024 17:35:34 -0800 Subject: [PATCH] aio: using stopped aios return NNG_ECLOSED rather than NNG_ECANCELED This is important to help differentiate between a single canceled operation and the situation where the entire underlying object is stopped and cannot be reused. Includes updates to docs and tests. --- docs/ref/api/aio.md | 8 +++++++- docs/ref/api/errors.md | 2 +- docs/ref/migrate/nng1.md | 11 +++++++++++ src/core/aio.c | 4 ++-- src/core/aio.h | 6 ++++-- src/sp/protocol/bus0/bus_test.c | 4 ++-- src/sp/protocol/pair0/pair0_test.c | 2 +- src/sp/protocol/pair1/pair1_test.c | 2 +- src/sp/protocol/pipeline0/pull_test.c | 2 +- src/sp/protocol/pipeline0/push_test.c | 2 +- src/sp/protocol/pubsub0/pub_test.c | 2 +- src/sp/protocol/pubsub0/sub_test.c | 2 +- src/sp/protocol/pubsub0/xsub_test.c | 2 +- src/sp/protocol/reqrep0/rep_test.c | 2 +- src/sp/protocol/reqrep0/xrep_test.c | 2 +- src/sp/protocol/reqrep0/xreq_test.c | 2 +- src/sp/protocol/survey0/respond_test.c | 2 +- src/sp/protocol/survey0/xrespond_test.c | 2 +- src/sp/protocol/survey0/xsurvey_test.c | 2 +- 19 files changed, 40 insertions(+), 21 deletions(-) diff --git a/docs/ref/api/aio.md b/docs/ref/api/aio.md index 230744e88..05e2fe02d 100644 --- a/docs/ref/api/aio.md +++ b/docs/ref/api/aio.md @@ -100,7 +100,7 @@ then [`nng_aio_result`] will return _err_. The {{i:`nng_aio_cancel`}} function acts like `nng_aio_abort`, but uses the error code [`NNG_ECANCELED`]{{hi:`NNG_ECANCELED`}}. -The {{i:`nng_aio_stop`}} function aborts the _aio_ operation with [`NNG_ECANCELED`], +The {{i:`nng_aio_stop`}} function aborts the _aio_ operation with [`NNG_ECLOSED`], and then waits the operation and any associated callback to complete. This function also marks _aio_ itself permanently stopped, so that any new operations scheduled by I/O providers using [`nng_aio_begin`] @@ -187,6 +187,12 @@ Operations that do not transfer data, or do not keep a count, may return zero fo > Either call these from the handle's completion callback, or after waiting for the > operation to complete with [`nng_aio_wait`]. +> [!IMPORTANT] +> If the return value from `nng_aio_result` is [`NNG_ECLOSED`], then the underlyng object +> or the _aio_ has been closed, and any further attempts to submit operations against it +> will fail immediately with `NNG_ECLOSED`, resulting in an infinite loop. To prevent this, +> do not resubmit an operation once the `NNG_ECLOSED` status is obtained. + ## Messages ```c diff --git a/docs/ref/api/errors.md b/docs/ref/api/errors.md index 6548c2634..13ef6435d 100644 --- a/docs/ref/api/errors.md +++ b/docs/ref/api/errors.md @@ -44,7 +44,7 @@ future locale-specific strings may be presented instead. | `NNG_EBUSY` | 4 | Resource busy. | | `NNG_ETIMEDOUT` | 5 | Timed out. The operation took longer than the allotted time. | | `NNG_ECONNREFUSED` | 6 | Connection refused. Usually indicates the wrong address or a server is running. | -| `NNG_ECLOSED` | 7 | Object closed. Typically the [socket] is closed. | +| `NNG_ECLOSED` | 7 | Object closed. Typically the [socket] or other object is closed. | | `NNG_EAGAIN` | 8 | Try again. Typcally for a non-blocking operation that might succeed later. | | `NNG_ENOTSUP` | 9 | Not supported. Perhaps the protocol or transport is not supported, or the operation is not not supported with the transport or protocol. | | `NNG_EADDRINUSE` | 10 | Address in use. The network address is already used by another process. Most often this is seen for [listeners][listener]. | diff --git a/docs/ref/migrate/nng1.md b/docs/ref/migrate/nng1.md index bff5b5815..ca74ecaa5 100644 --- a/docs/ref/migrate/nng1.md +++ b/docs/ref/migrate/nng1.md @@ -38,6 +38,17 @@ The `NNG_OPT_WSS_REQUEST_HEADERS` and `NNG_OPT_WSS_RESPONSE_HEADERS` aliases for Just convert any use of them to `NNG_OPT_WS_REQUEST_HEADERS` or `NNG_OPT_WS_RESPONSE_HEADERS` as appropriate. +## Asynchronous I/O Returns + +When closing an [`nng_aio`] object with [`nng_aio_stop`], the result +of operation will be aborted with a result error of [`NNG_ECLOSED`], instead of [`NNG_ECANCELED`]. +The `NNG_ECLOSED` error is a permanent failure, and operations should not be tried when +encountering it, whereas `NNG_ECANCELED` might just apply to a single operation that was +canceled by the submitter. + +This situation can also occur when an underlying object such as a socket is closed. +In all cases, `NNG_ECLOSED` should be treated as a permanent failure. + ## TLS Configuration The support for configuring TLS via `NNG_OPT_TLS_CONFIG`, `NNG_TLS_AUTH_MODE`, `NNG_OPT_TLS_CA_FILE`, diff --git a/src/core/aio.c b/src/core/aio.c index 02c9410bf..571a34aaf 100644 --- a/src/core/aio.c +++ b/src/core/aio.c @@ -350,14 +350,14 @@ nni_aio_begin(nni_aio *aio) // We should not reschedule anything at this point. if (aio->a_stop || eq->eq_stop) { - aio->a_result = NNG_ECANCELED; + aio->a_result = NNG_ECLOSED; aio->a_cancel_fn = NULL; aio->a_expire = NNI_TIME_NEVER; aio->a_sleep = false; aio->a_expire_ok = false; nni_mtx_unlock(&eq->eq_mtx); nni_task_dispatch(&aio->a_task); - return (NNG_ECANCELED); + return (NNG_ECLOSED); } nni_task_prep(&aio->a_task); nni_mtx_unlock(&eq->eq_mtx); diff --git a/src/core/aio.h b/src/core/aio.h index 9491a2fa9..96c1bafdd 100644 --- a/src/core/aio.h +++ b/src/core/aio.h @@ -131,10 +131,12 @@ extern void nni_aio_abort(nni_aio *, int rv); // nni_aio_begin is called by a provider to indicate it is starting the // operation, and to check that the aio has not already been marked for -// teardown. It returns 0 on success, or NNG_ECANCELED if the aio is being +// teardown. It returns 0 on success, or NNG_ECLOSED if the aio is being // torn down. (In that case, no operation should be aborted without any // call to any other functions on this AIO, most especially not the -// nng_aio_finish family of functions.) +// nng_aio_finish family of functions.) Note that it also executes the +// completion callback with NNG_ECLOSED. It is important that aio consumers +// do not resubmit an operation if NNG_ECLOSED is returned. extern int nni_aio_begin(nni_aio *); extern void *nni_aio_get_prov_data(nni_aio *); diff --git a/src/sp/protocol/bus0/bus_test.c b/src/sp/protocol/bus0/bus_test.c index a6b04df98..aeb585299 100644 --- a/src/sp/protocol/bus0/bus_test.c +++ b/src/sp/protocol/bus0/bus_test.c @@ -189,12 +189,12 @@ test_bus_aio_stopped(void) nng_recv_aio(s1, aio); nng_aio_wait(aio); - NUTS_FAIL(nng_aio_result(aio), NNG_ECANCELED); + NUTS_FAIL(nng_aio_result(aio), NNG_ECLOSED); nng_aio_set_msg(aio, msg); nng_send_aio(s1, aio); nng_aio_wait(aio); - NUTS_FAIL(nng_aio_result(aio), NNG_ECANCELED); + NUTS_FAIL(nng_aio_result(aio), NNG_ECLOSED); nng_aio_free(aio); nng_msg_free(msg); diff --git a/src/sp/protocol/pair0/pair0_test.c b/src/sp/protocol/pair0/pair0_test.c index b01bdf91f..4eadf3637 100644 --- a/src/sp/protocol/pair0/pair0_test.c +++ b/src/sp/protocol/pair0/pair0_test.c @@ -212,7 +212,7 @@ test_pair0_send_closed_aio(void) nng_aio_stop(aio); nng_send_aio(s1, aio); nng_aio_wait(aio); - NUTS_FAIL(nng_aio_result(aio), NNG_ECANCELED); + NUTS_FAIL(nng_aio_result(aio), NNG_ECLOSED); nng_msg_free(msg); nng_aio_free(aio); NUTS_PASS(nng_close(s1)); diff --git a/src/sp/protocol/pair1/pair1_test.c b/src/sp/protocol/pair1/pair1_test.c index 8d2ad9407..13e7a3a30 100644 --- a/src/sp/protocol/pair1/pair1_test.c +++ b/src/sp/protocol/pair1/pair1_test.c @@ -278,7 +278,7 @@ test_pair1_send_closed_aio(void) nng_aio_stop(aio); nng_send_aio(s1, aio); nng_aio_wait(aio); - NUTS_FAIL(nng_aio_result(aio), NNG_ECANCELED); + NUTS_FAIL(nng_aio_result(aio), NNG_ECLOSED); nng_msg_free(msg); nng_aio_free(aio); NUTS_PASS(nng_close(s1)); diff --git a/src/sp/protocol/pipeline0/pull_test.c b/src/sp/protocol/pipeline0/pull_test.c index 74fd60466..aeaddefa8 100644 --- a/src/sp/protocol/pipeline0/pull_test.c +++ b/src/sp/protocol/pipeline0/pull_test.c @@ -175,7 +175,7 @@ test_pull_recv_aio_stopped(void) nng_aio_stop(aio); nng_recv_aio(s, aio); nng_aio_wait(aio); - NUTS_FAIL(nng_aio_result(aio), NNG_ECANCELED); + NUTS_FAIL(nng_aio_result(aio), NNG_ECLOSED); NUTS_CLOSE(s); nng_aio_free(aio); } diff --git a/src/sp/protocol/pipeline0/push_test.c b/src/sp/protocol/pipeline0/push_test.c index 5eb988448..8aa9bb817 100644 --- a/src/sp/protocol/pipeline0/push_test.c +++ b/src/sp/protocol/pipeline0/push_test.c @@ -252,7 +252,7 @@ test_push_send_aio_stopped(void) nng_aio_stop(aio); nng_send_aio(s, aio); nng_aio_wait(aio); - NUTS_FAIL(nng_aio_result(aio), NNG_ECANCELED); + NUTS_FAIL(nng_aio_result(aio), NNG_ECLOSED); NUTS_CLOSE(s); nng_aio_free(aio); nng_msg_free(m); diff --git a/src/sp/protocol/pubsub0/pub_test.c b/src/sp/protocol/pubsub0/pub_test.c index a2a20dd7c..abc39768c 100644 --- a/src/sp/protocol/pubsub0/pub_test.c +++ b/src/sp/protocol/pubsub0/pub_test.c @@ -191,7 +191,7 @@ test_sub_ctx_recv_aio_stopped(void) nng_aio_stop(aio); nng_ctx_recv(ctx, aio); nng_aio_wait(aio); - NUTS_FAIL(nng_aio_result(aio), NNG_ECANCELED); + NUTS_FAIL(nng_aio_result(aio), NNG_ECLOSED); NUTS_PASS(nng_ctx_close(ctx)); NUTS_CLOSE(sub); nng_aio_free(aio); diff --git a/src/sp/protocol/pubsub0/sub_test.c b/src/sp/protocol/pubsub0/sub_test.c index 74a547f41..8bf65350f 100644 --- a/src/sp/protocol/pubsub0/sub_test.c +++ b/src/sp/protocol/pubsub0/sub_test.c @@ -223,7 +223,7 @@ test_sub_ctx_recv_aio_stopped(void) nng_aio_stop(aio); nng_ctx_recv(ctx, aio); nng_aio_wait(aio); - NUTS_FAIL(nng_aio_result(aio), NNG_ECANCELED); + NUTS_FAIL(nng_aio_result(aio), NNG_ECLOSED); NUTS_PASS(nng_ctx_close(ctx)); NUTS_CLOSE(sub); nng_aio_free(aio); diff --git a/src/sp/protocol/pubsub0/xsub_test.c b/src/sp/protocol/pubsub0/xsub_test.c index eb918e145..002bb602e 100644 --- a/src/sp/protocol/pubsub0/xsub_test.c +++ b/src/sp/protocol/pubsub0/xsub_test.c @@ -337,7 +337,7 @@ test_xsub_recv_aio_stopped(void) nng_aio_stop(aio); nng_recv_aio(sub, aio); nng_aio_wait(aio); - NUTS_FAIL(nng_aio_result(aio), NNG_ECANCELED); + NUTS_FAIL(nng_aio_result(aio), NNG_ECLOSED); NUTS_CLOSE(sub); nng_aio_free(aio); } diff --git a/src/sp/protocol/reqrep0/rep_test.c b/src/sp/protocol/reqrep0/rep_test.c index 579f795c1..a05c0f339 100644 --- a/src/sp/protocol/reqrep0/rep_test.c +++ b/src/sp/protocol/reqrep0/rep_test.c @@ -369,7 +369,7 @@ test_rep_ctx_recv_aio_stopped(void) nng_aio_stop(aio); nng_ctx_recv(ctx, aio); nng_aio_wait(aio); - NUTS_FAIL(nng_aio_result(aio), NNG_ECANCELED); + NUTS_FAIL(nng_aio_result(aio), NNG_ECLOSED); NUTS_PASS(nng_ctx_close(ctx)); NUTS_CLOSE(rep); nng_aio_free(aio); diff --git a/src/sp/protocol/reqrep0/xrep_test.c b/src/sp/protocol/reqrep0/xrep_test.c index d5110469f..c179282c8 100644 --- a/src/sp/protocol/reqrep0/xrep_test.c +++ b/src/sp/protocol/reqrep0/xrep_test.c @@ -262,7 +262,7 @@ test_xrep_recv_aio_stopped(void) nng_aio_stop(aio); nng_recv_aio(rep, aio); nng_aio_wait(aio); - NUTS_FAIL(nng_aio_result(aio), NNG_ECANCELED); + NUTS_FAIL(nng_aio_result(aio), NNG_ECLOSED); NUTS_CLOSE(rep); nng_aio_free(aio); } diff --git a/src/sp/protocol/reqrep0/xreq_test.c b/src/sp/protocol/reqrep0/xreq_test.c index 28f381fec..442104127 100644 --- a/src/sp/protocol/reqrep0/xreq_test.c +++ b/src/sp/protocol/reqrep0/xreq_test.c @@ -168,7 +168,7 @@ test_xreq_recv_aio_stopped(void) nng_aio_stop(aio); nng_recv_aio(req, aio); nng_aio_wait(aio); - NUTS_FAIL(nng_aio_result(aio), NNG_ECANCELED); + NUTS_FAIL(nng_aio_result(aio), NNG_ECLOSED); NUTS_CLOSE(req); nng_aio_free(aio); } diff --git a/src/sp/protocol/survey0/respond_test.c b/src/sp/protocol/survey0/respond_test.c index 0260dc30a..3d86fdcc2 100644 --- a/src/sp/protocol/survey0/respond_test.c +++ b/src/sp/protocol/survey0/respond_test.c @@ -257,7 +257,7 @@ test_resp_ctx_recv_aio_stopped(void) nng_aio_stop(aio); nng_ctx_recv(ctx, aio); nng_aio_wait(aio); - NUTS_FAIL(nng_aio_result(aio), NNG_ECANCELED); + NUTS_FAIL(nng_aio_result(aio), NNG_ECLOSED); NUTS_PASS(nng_ctx_close(ctx)); NUTS_CLOSE(resp); nng_aio_free(aio); diff --git a/src/sp/protocol/survey0/xrespond_test.c b/src/sp/protocol/survey0/xrespond_test.c index 8106f161f..a3050177d 100644 --- a/src/sp/protocol/survey0/xrespond_test.c +++ b/src/sp/protocol/survey0/xrespond_test.c @@ -262,7 +262,7 @@ test_xresp_recv_aio_stopped(void) nng_aio_stop(aio); nng_recv_aio(resp, aio); nng_aio_wait(aio); - NUTS_FAIL(nng_aio_result(aio), NNG_ECANCELED); + NUTS_FAIL(nng_aio_result(aio), NNG_ECLOSED); NUTS_CLOSE(resp); nng_aio_free(aio); } diff --git a/src/sp/protocol/survey0/xsurvey_test.c b/src/sp/protocol/survey0/xsurvey_test.c index e90939e97..8567e421e 100644 --- a/src/sp/protocol/survey0/xsurvey_test.c +++ b/src/sp/protocol/survey0/xsurvey_test.c @@ -167,7 +167,7 @@ test_xsurvey_recv_aio_stopped(void) nng_aio_stop(aio); nng_recv_aio(surv, aio); nng_aio_wait(aio); - NUTS_FAIL(nng_aio_result(aio), NNG_ECANCELED); + NUTS_FAIL(nng_aio_result(aio), NNG_ECLOSED); NUTS_CLOSE(surv); nng_aio_free(aio); }