From f6289afc9765478bc02e1fb4530fd395d0509b35 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= <ondrej@sury.org>
Date: Wed, 5 May 2021 14:25:42 +0200
Subject: [PATCH] Add reference counting for enqueued tasks

Previously, taskmgr was using ISC_LIST for keeping the enqueued tasks
and the ISC_LIST have an inherent property that the task would not get
enqueued twice.  When taskmgr switched to using netmgr, we are using
isc_queue_t which can enqueue same item multiple times, and this was
causing problem when the previous isc_task_run() would find the task to
be empty and shutting down.  It would directly destroy the task and
leave the following netievents with already freed task.

This commit adds references counting around task_ready(++) and
task_run(--) and adds isc_task_ready() public API wrapper around
task_ready, so we can call it from netmgr when the event quantum would
be reached.
---
 lib/isc/include/isc/task.h |  6 ++++++
 lib/isc/netmgr/netmgr.c    |  3 +--
 lib/isc/task.c             | 17 ++++++++++++++---
 3 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/lib/isc/include/isc/task.h b/lib/isc/include/isc/task.h
index f8a790403e..45d062ee32 100644
--- a/lib/isc/include/isc/task.h
+++ b/lib/isc/include/isc/task.h
@@ -136,6 +136,12 @@ isc_task_create_bound(isc_taskmgr_t *manager, unsigned int quantum,
  *\li	#ISC_R_SHUTTINGDOWN
  */
 
+void
+isc_task_ready(isc_task_t *task);
+/*%<
+ * Enqueue the task onto netmgr queue.
+ */
+
 isc_result_t
 isc_task_run(isc_task_t *task);
 /*%<
diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c
index 74252cb546..89955f0a11 100644
--- a/lib/isc/netmgr/netmgr.c
+++ b/lib/isc/netmgr/netmgr.c
@@ -800,8 +800,7 @@ isc__nm_async_task(isc__networker_t *worker, isc__netievent_t *ev0) {
 
 	switch (result) {
 	case ISC_R_QUOTA:
-		isc_nm_task_enqueue(worker->mgr, (isc_task_t *)ievent->task,
-				    isc_nm_tid());
+		isc_task_ready(ievent->task);
 		return;
 	case ISC_R_SUCCESS:
 		return;
diff --git a/lib/isc/task.c b/lib/isc/task.c
index e8210d149a..ec002a5563 100644
--- a/lib/isc/task.c
+++ b/lib/isc/task.c
@@ -103,6 +103,7 @@ struct isc_task {
 	task_state_t state;
 	int pause_cnt;
 	isc_refcount_t references;
+	isc_refcount_t running;
 	isc_eventlist_t events;
 	isc_eventlist_t on_shutdown;
 	unsigned int nevents;
@@ -179,6 +180,7 @@ task_finished(isc_task_t *task) {
 
 	XTRACE("task_finished");
 
+	isc_refcount_destroy(&task->running);
 	isc_refcount_destroy(&task->references);
 
 	LOCK(&manager->lock);
@@ -237,6 +239,7 @@ isc_task_create_bound(isc_taskmgr_t *manager, unsigned int quantum,
 	task->pause_cnt = 0;
 
 	isc_refcount_init(&task->references, 1);
+	isc_refcount_init(&task->running, 0);
 	INIT_LIST(task->events);
 	INIT_LIST(task->on_shutdown);
 	task->nevents = 0;
@@ -337,9 +340,16 @@ task_ready(isc_task_t *task) {
 	REQUIRE(VALID_MANAGER(manager));
 
 	XTRACE("task_ready");
+
+	isc_refcount_increment0(&task->running);
 	isc_nm_task_enqueue(manager->netmgr, task, task->threadid);
 }
 
+void
+isc_task_ready(isc_task_t *task) {
+	task_ready(task);
+}
+
 static inline bool
 task_detach(isc_task_t *task) {
 	/*
@@ -868,13 +878,13 @@ task_run(isc_task_t *task) {
 			 */
 			XTRACE("empty");
 			if (isc_refcount_current(&task->references) == 0 &&
-			    TASK_SHUTTINGDOWN(task)) {
+			    TASK_SHUTTINGDOWN(task))
+			{
 				/*
 				 * The task is done.
 				 */
 				XTRACE("done");
 				finished = true;
-				task->state = task_state_done;
 			} else {
 				if (task->state == task_state_running) {
 					XTRACE("idling");
@@ -909,7 +919,8 @@ task_run(isc_task_t *task) {
 	}
 	UNLOCK(&task->lock);
 
-	if (finished) {
+	if (isc_refcount_decrement(&task->running) == 1 && finished) {
+		task->state = task_state_done;
 		task_finished(task);
 	}