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.

4 files changed, 13 insertions, 29 deletions

diff --git a/src/core/aio.c b/src/core/aio.c
index d003c251..eaff5c80 100644
--- a/src/core/aio.c
+++ b/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);
 		}
 	}
 }
diff --git a/src/core/aio.h b/src/core/aio.h
index 5f7ddb69..9491a2fa 100644
--- a/src/core/aio.h
+++ b/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;
 
diff --git a/src/core/taskq.c b/src/core/taskq.c
index 1f0ae1b6..53252a8e 100644
--- a/src/core/taskq.c
+++ b/src/core/taskq.c
@@ -199,20 +199,6 @@ nni_task_prep(nni_task *task)
 }
 
 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)
 {
 	nni_mtx_lock(&task->task_mtx);
diff --git a/src/core/taskq.h b/src/core/taskq.h
index d3299433..356ef725 100644
--- a/src/core/taskq.h
+++ b/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 *);