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.
nanomsg · Dec 9, 2024 · 0f0cc86 · 0f0cc86
1 parent b4b438c
commit 0f0cc86
Show file tree

Hide file tree

Showing 19 changed files with 40 additions and 21 deletions.
diff --git 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
@@ -44,7 +44,7 @@ future locale-specific strings may be presented instead.
 | `NNG_EBUSY`<a name="NNG_EBUSY"></a>               | 4                       | Resource busy.                                                                                                                                    |
 | `NNG_ETIMEDOUT`<a name="NNG_ETIMEDOUT"></a>       | 5                       | Timed out. The operation took longer than the allotted time.                                                                                      |
 | `NNG_ECONNREFUSED`<a name="NNG_ECONNREFUSED"></a> | 6                       | Connection refused. Usually indicates the wrong address or a server is running.                                                                   |
-| `NNG_ECLOSED`<a name="NNG_ECLOSED"></a>           | 7                       | Object closed. Typically the [socket] is closed.                                                                                                  |
+| `NNG_ECLOSED`<a name="NNG_ECLOSED"></a>           | 7                       | Object closed. Typically the [socket] or other object is closed.                                                                                  |
 | `NNG_EAGAIN`<a name="NNG_EAGAIN"></a>             | 8                       | Try again. Typcally for a non-blocking operation that might succeed later.                                                                        |
 | `NNG_ENOTSUP`<a name="NNG_ENOTSUP"></a>           | 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`<a name="NNG_EADDRINUSE"></a>     | 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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);
 }