aio: task_abort was a mistake

The use of task_abort to prematurely fail an aio at scheduling
time was a mistake, because it could have led to duplicate calls
to nng_aio_finish().  We do need to ensure that we leave an indicator
so that nni_aio_schedule can return the abort status to caller,
in the case that abort is called between the nni_aio_begin and
nni_aio_schedule calls.

include/nng/nng.h

@@ -543,6 +543,8 @@ NNG_DECL void nng_aio_reap(nng_aio *);
 // AIO to be free, including for the callback to have completed
 // execution.  Therefore, the caller must NOT hold any locks that
 // are acquired in the callback, or deadlock will occur.
+// No further operations may be scheduled on the aio, stop is
+// a permanent operation.
 NNG_DECL void nng_aio_stop(nng_aio *);
 
 // nng_aio_result returns the status/result of the operation. This
@@ -558,6 +560,7 @@ NNG_DECL size_t nng_aio_count(nng_aio *);
 // nng_aio_cancel attempts to cancel any in-progress I/O operation.
 // The AIO callback will still be executed, but if the cancellation is
 // successful then the status will be NNG_ECANCELED.
+// An AIO can only be canceled if it was submitted already.
 NNG_DECL void nng_aio_cancel(nng_aio *);
 
 // nng_aio_abort is like nng_aio_cancel, but allows for a different

src/core/aio.c

@@ -118,8 +118,6 @@ nni_aio_fini(nni_aio *aio)
 
 		if (fn != NULL) {
 			fn(aio, arg, NNG_ECLOSED);
-		} else {
-			nni_task_abort(&aio->a_task);
 		}
 
 		nni_task_fini(&aio->a_task);
@@ -203,8 +201,6 @@ nni_aio_stop(nni_aio *aio)
 
 		if (fn != NULL) {
 			fn(aio, arg, NNG_ECANCELED);
-		} else {
-			nni_task_abort(&aio->a_task);
 		}
 
 		nni_aio_wait(aio);
@@ -230,8 +226,6 @@ nni_aio_close(nni_aio *aio)
 
 		if (fn != NULL) {
 			fn(aio, arg, NNG_ECLOSED);
-		} else {
-			nni_task_abort(&aio->a_task);
 		}
 	}
 }
@@ -352,6 +346,7 @@ nni_aio_begin(nni_aio *aio)
 	aio->a_result    = 0;
 	aio->a_count     = 0;
 	aio->a_cancel_fn = NULL;
+	aio->a_abort     = false;
 
 	// We should not reschedule anything at this point.
 	if (aio->a_stop || eq->eq_stop) {
@@ -378,7 +373,6 @@ nni_aio_schedule(nni_aio *aio, nni_aio_cancel_fn cancel, void *data)
 		// Convert the relative timeout to an absolute timeout.
 		switch (aio->a_timeout) {
 		case NNG_DURATION_ZERO:
-			nni_task_abort(&aio->a_task);
 			return (NNG_ETIMEDOUT);
 		case NNG_DURATION_INFINITE:
 		case NNG_DURATION_DEFAULT:
@@ -391,8 +385,12 @@ nni_aio_schedule(nni_aio *aio, nni_aio_cancel_fn cancel, void *data)
 	}
 
 	nni_mtx_lock(&eq->eq_mtx);
+	if (aio->a_abort) {
+		int rv = aio->a_result;
+		nni_mtx_unlock(&eq->eq_mtx);
+		return (rv);
+	}
 	if (aio->a_stop || eq->eq_stop) {
-		nni_task_abort(&aio->a_task);
 		nni_mtx_unlock(&eq->eq_mtx);
 		return (NNG_ECLOSED);
 	}
@@ -426,13 +424,17 @@ nni_aio_abort(nni_aio *aio, int rv)
 		arg               = aio->a_cancel_arg;
 		aio->a_cancel_fn  = NULL;
 		aio->a_cancel_arg = NULL;
+		if (fn == NULL) {
+			// We haven't been scheduled yet,
+			// so make sure that schedule will abort.
+			aio->a_abort  = true;
+			aio->a_result = rv;
+		}
 		nni_mtx_unlock(&eq->eq_mtx);
 
 		// Stop any I/O at the provider level.
 		if (fn != NULL) {
 			fn(aio, arg, rv);
-		} else {
-			nni_task_abort(&aio->a_task);
 		}
 	}
 }

src/core/aio.h

@@ -214,6 +214,7 @@ struct nng_aio {
 	bool         a_expire_ok;  // Expire from sleep is ok
 	bool         a_expiring;   // Expiration in progress
 	bool         a_use_expire; // Use expire instead of timeout
+	bool         a_abort;      // Task was aborted
 	bool         a_init;       // Initialized this
 	nni_task     a_task;
 

src/core/taskq.c

@@ -198,20 +198,6 @@ nni_task_prep(nni_task *task)
 	nni_mtx_unlock(&task->task_mtx);
 }
 
-void
-nni_task_abort(nni_task *task)
-{
-	// This is called when unscheduling the task.
-	nni_mtx_lock(&task->task_mtx);
-	if (task->task_prep) {
-		task->task_prep = false;
-		task->task_busy--;
-		if (task->task_busy == 0) {
-			nni_cv_wake(&task->task_cv);
-		}
-	}
-	nni_mtx_unlock(&task->task_mtx);
-}
 void
 nni_task_wait(nni_task *task)
 {

src/core/taskq.h

@@ -40,11 +40,6 @@ extern void nni_task_exec(nni_task *);
 // nni_task_exec).
 extern void nni_task_prep(nni_task *);
 
-// nni_task_abort is called to undo the effect of nni_task_prep,
-// basically. The aio framework uses this when nni_aio_schedule()
-// returns an error.
-extern void nni_task_abort(nni_task *);
-
 // nni_task_busy checks to see if a task is still busy.
 // This is uses the same check that nni_task_wait uses.
 extern bool nni_task_busy(nni_task *);