From fd80ac9b3ea3c6e213236e27ae8b73856d2a5158 Mon Sep 17 00:00:00 2001
From: David Härdeman <david@hardeman.nu>
Date: Wed, 10 Jun 2020 22:53:42 +0200
Subject: Improve refcounting

---
 announce.c |  5 ++---
 cfgdir.c   |  4 +---
 idle.c     |  6 ++----
 main.c     | 12 ++++++++++--
 main.h     | 12 ++++++------
 proxy.c    | 12 +++---------
 rcon.c     | 29 ++++++++++++++++++++++++++---
 server.c   |  9 +++++----
 uring.c    | 53 +++++++++++++++++++++++++++++++++++++++++------------
 uring.h    |  4 +++-
 10 files changed, 99 insertions(+), 47 deletions(-)

diff --git a/announce.c b/announce.c
index 797807b..3a50771 100644
--- a/announce.c
+++ b/announce.c
@@ -78,7 +78,6 @@ announce_cb(struct cfg *cfg, struct uring_task *task, int res)
 
 	if (task->dead) {
 		fprintf(stderr, "%s: task is dead\n", __func__);
-		uring_task_put(cfg, task);
 		return;
 	}
 
@@ -118,8 +117,8 @@ announce_delete(struct cfg *cfg)
 	}
 
 	fprintf(stderr, "%s called, closing fd %i\n", __func__, cfg->aev->task.fd);
-	uring_cancel(cfg, &cfg->aev->task);
-	uring_task_put(cfg, &cfg->aev->mcast_task);
+	uring_task_destroy(cfg, &cfg->aev->mcast_task);
+	uring_task_destroy(cfg, &cfg->aev->task);
 	cfg->aev = NULL;
 }
 
diff --git a/cfgdir.c b/cfgdir.c
index 4322fec..9ffff3e 100644
--- a/cfgdir.c
+++ b/cfgdir.c
@@ -289,7 +289,6 @@ inotify_free(struct uring_task *task)
 
 	xfree(iev);
 	cfg->iev = NULL;
-	uring_task_put(cfg, &cfg->task);
 }
 
 static void
@@ -346,7 +345,6 @@ inotify_cb(struct cfg *cfg, struct uring_task *task, int res)
 
 	if (task->dead) {
 		fprintf(stderr, "%s: task is dead\n", __func__);
-		uring_task_put(cfg, task);
 		return;
 	}
 
@@ -398,7 +396,7 @@ cfgdir_delete(struct cfg *cfg)
 	}
 
 	fprintf(stderr, "%s called, closing fd %i\n", __func__, cfg->iev->task.fd);
-	uring_cancel(cfg, &cfg->iev->task);
+	uring_task_destroy(cfg, &cfg->iev->task);
 	cfg->iev = NULL;
 }
 
diff --git a/idle.c b/idle.c
index 5a60973..03d9f88 100644
--- a/idle.c
+++ b/idle.c
@@ -376,7 +376,6 @@ idle_cb(struct cfg *cfg, struct uring_task *task, int res)
 
 	if (task->dead) {
 		fprintf(stderr, "%s: task is dead\n", __func__);
-		uring_task_put(cfg, task);
 		return;
 	}
 
@@ -422,9 +421,8 @@ idle_delete(struct cfg *cfg, struct server *server)
 		return;
 
 	fprintf(stderr, "%s called, closing fd %i\n", __func__, idle->task.fd);
-	uring_cancel(cfg, &idle->task);
-	uring_task_put(cfg, &idle->task);
-	uring_task_put(cfg, &idle->idlecheck);
+	uring_task_destroy(cfg, &idle->idlecheck);
+	uring_task_destroy(cfg, &idle->task);
 	server->idle = NULL;
 }
 
diff --git a/main.c b/main.c
index c8e8a8d..0f41ea0 100644
--- a/main.c
+++ b/main.c
@@ -184,6 +184,11 @@ signalfd_read(struct cfg *cfg, struct uring_task *task, int res)
 	struct server *server, *stmp;
 	static int count = 0;
 
+	if (task->dead) {
+		fprintf(stderr, "%s: task dead\n", __func__);
+		return;
+	}
+
 	count++;
 	if (count > 5)
 		exit(EXIT_FAILURE);
@@ -197,14 +202,17 @@ signalfd_read(struct cfg *cfg, struct uring_task *task, int res)
 	} else {
 		fprintf(stderr, "Got a signal to dump tree\n");
 		dump_tree(cfg);
+		uring_task_put(cfg, &sev->task);
 		announce_delete(cfg);
 		cfgdir_delete(cfg);
 		list_for_each_entry_safe(server, stmp, &cfg->servers, list)
 			server_delete(cfg, server);
 		fprintf(stderr, "%s: putting sev task 0x%p\n", __func__, &sev->task);
-		uring_task_put(cfg, &sev->task);
-		//uring_read(cfg, &sev->task, &sev->buf, sizeof(sev->buf), signalfd_read);
+		uring_delete(cfg);
+		return;
 	}
+
+	uring_read(cfg, &sev->task, &sev->buf, sizeof(sev->buf), signalfd_read);
 }
 
 static int hack_efd = -1;
diff --git a/main.h b/main.h
index a3b5512..d0cb41b 100644
--- a/main.h
+++ b/main.h
@@ -34,15 +34,15 @@ struct uring_task_buf {
 
 struct uring_task {
 	const char *name;
-        unsigned refcount;
+	unsigned refcount;
 	int fd;
-        void *parent;
-        void (*free)(struct uring_task *);
+	struct uring_task *parent;
+	void (*free)(struct uring_task *);
 	bool dead;
 	struct uring_task_buf *tbuf;
-        callback_t callback;
-        rcallback_t complete_callback; /* to check if tbuf processing is done */
-        callback_t final_callback; /* once tbuf processing is done */
+	callback_t callback;
+	rcallback_t complete_callback; /* to check if tbuf processing is done */
+	callback_t final_callback; /* once tbuf processing is done */
 };
 
 struct cfg {
diff --git a/proxy.c b/proxy.c
index 38f5cea..eaa831b 100644
--- a/proxy.c
+++ b/proxy.c
@@ -103,15 +103,9 @@ proxy_delete(struct cfg *cfg, struct server_proxy *proxy)
 	fprintf(stderr, "%s: shutting down proxy 0x%p\n", __func__, proxy);
 
 	/* FIXME: review half-open proxy situation */
-	if (proxy->servertask.fd >= 0)
-		uring_cancel(cfg, &proxy->servertask);
-
-	if (proxy->clienttask.fd >= 0)
-		uring_cancel(cfg, &proxy->clienttask);
-
-	uring_task_put(cfg, &proxy->servertask);
-	uring_task_put(cfg, &proxy->clienttask);
-	uring_task_put(cfg, &proxy->task);
+	uring_task_destroy(cfg, &proxy->servertask);
+	uring_task_destroy(cfg, &proxy->clienttask);
+	uring_task_destroy(cfg, &proxy->task);
 }
 
 static void proxy_client_data_in(struct cfg *cfg, struct uring_task *task, int res);
diff --git a/rcon.c b/rcon.c
index be2f279..02fd021 100644
--- a/rcon.c
+++ b/rcon.c
@@ -52,9 +52,7 @@ rcon_delete(struct cfg *cfg, struct server *server)
 		return;
 
 	fprintf(stderr, "%s called, closing fd %i\n", __func__, rcon->task.fd);
-	uring_cancel(cfg, &rcon->task);
-	/* FIXME: Won't the refcount be wrong? */
-	uring_task_put(cfg, &rcon->task);
+	uring_task_destroy(cfg, &rcon->task);
 	server->rcon = NULL;
 }
 
@@ -213,6 +211,11 @@ rcon_stop_reply(struct cfg *cfg, struct uring_task *task, int res)
 	char *msg;
 
 	fprintf(stderr, "%s: result %i\n", __func__, res);
+	if (task->dead) {
+		fprintf(stderr, "%s: task dead\n", __func__);
+		return;
+	}
+
 	if (res < 0)
 		goto out;
 
@@ -237,6 +240,11 @@ rcon_stop_sent(struct cfg *cfg, struct uring_task *task, int res)
 	struct rcon *rcon = container_of(task, struct rcon, task);
 
 	fprintf(stderr, "%s: result %i\n", __func__, res);
+	if (task->dead) {
+		fprintf(stderr, "%s: task dead\n", __func__);
+		return;
+	}
+
 	if (res < 0) {
 		uring_task_put(cfg, &rcon->task);
 		return;
@@ -255,6 +263,11 @@ rcon_login_reply(struct cfg *cfg, struct uring_task *task, int res)
 	char *msg;
 
 	fprintf(stderr, "%s: result %i\n", __func__, res);
+	if (task->dead) {
+		fprintf(stderr, "%s: task dead\n", __func__);
+		return;
+	}
+
 	if (res < 0)
 		goto out;
 
@@ -286,6 +299,11 @@ rcon_login_sent(struct cfg *cfg, struct uring_task *task, int res)
 	struct rcon *rcon = container_of(task, struct rcon, task);
 
 	fprintf(stderr, "%s: result %i\n", __func__, res);
+	if (task->dead) {
+		fprintf(stderr, "%s: task dead\n", __func__);
+		return;
+	}
+
 	if (res < 0) {
 		uring_task_put(cfg, &rcon->task);
 		return;
@@ -303,6 +321,11 @@ rcon_connected(struct cfg *cfg, struct uring_task *task, int res)
 	struct rcon *rcon = container_of(task, struct rcon, task);
 
 	fprintf(stderr, "%s: connected %i\n", __func__, res);
+	if (task->dead) {
+		fprintf(stderr, "%s: task dead\n", __func__);
+		return;
+	}
+
 	if (res < 0) {
 		rcon_connect_next_rcon(cfg, rcon);
 		return;
diff --git a/server.c b/server.c
index f317d36..7a0dc5b 100644
--- a/server.c
+++ b/server.c
@@ -89,7 +89,7 @@ server_delete(struct cfg *cfg, struct server *scfg)
 	rcon_delete(cfg, scfg);
 
 	list_for_each_entry_safe(local, ltmp, &scfg->locals, list)
-		uring_cancel(cfg, &local->task);
+		uring_task_destroy(cfg, &local->task);
 
 	list_for_each_entry_safe(proxy, ptmp, &scfg->proxys, list)
 		proxy_delete(cfg, proxy);
@@ -106,7 +106,7 @@ server_delete(struct cfg *cfg, struct server *scfg)
 
 	uring_poll_cancel(cfg, &scfg->exec_task);
 	uring_task_put(cfg, &scfg->exec_task);
-	uring_task_put(cfg, &scfg->task);
+	uring_task_destroy(cfg, &scfg->task);
 }
 
 void
@@ -183,7 +183,6 @@ server_local_accept(struct cfg *cfg, struct uring_task *task, int res)
 
 	if (task->dead) {
 		fprintf(stderr, "Task dead!\n");
-		uring_task_put(cfg, task);
 		return;
 	}
 
@@ -269,8 +268,10 @@ server_exec_done(struct cfg *cfg, struct uring_task *task, int res)
 	int r;
 	siginfo_t info;
 
-	if (task->dead)
+	if (task->dead) {
+		/* Should we leave child processes running? */
 		goto out;
+	}
 
 	if (!(res & POLLIN)) {
 		fprintf(stderr, "%s: unexpected result: %i\n", __func__, res);
diff --git a/uring.c b/uring.c
index 347c7fc..26e2073 100644
--- a/uring.c
+++ b/uring.c
@@ -64,6 +64,32 @@ uring_task_refdump(struct uring_task *task)
 		task->refcount);
 }
 
+static void
+uring_cancel(struct cfg *cfg, struct uring_task *task)
+{
+	struct io_uring_sqe *sqe = get_sqe(cfg);
+	
+	io_uring_prep_cancel(sqe, task, 0);
+	io_uring_sqe_set_data(sqe, NULL);
+}
+
+/*
+ * Similar to uring_task_put, but can be called from other tasks
+ * while the task is active.
+ */
+void
+uring_task_destroy(struct cfg *cfg, struct uring_task *task)
+{
+	fprintf(stderr, "%s: called with task %s (%p), fd %i and refcount %u\n",
+		__func__, task->name, task, task->fd, task->refcount);
+
+	if (task->fd >= 0)
+		uring_cancel(cfg, task);
+
+	task->dead = true;
+	uring_task_put(cfg, task);
+}
+
 void
 uring_task_put(struct cfg *cfg, struct uring_task *task)
 {
@@ -85,6 +111,10 @@ uring_task_put(struct cfg *cfg, struct uring_task *task)
 		return;
 	}
 
+	if (parent)
+		fprintf(stderr, "%s: task %s (%p) putting parent %s (%p)\n",
+			__func__, task->name, task, task->parent->name, task->parent);
+
 	if (task->free)
 		task->free(task);
 
@@ -163,8 +193,11 @@ uring_task_init(struct uring_task *task, const char *name,
 	task->name = name;
 	task->tbuf = NULL;
 
-	if (task->parent)
+	if (task->parent) {
+		fprintf(stderr, "%s: task %s (%p) getting parent %s (%p)\n",
+			__func__, task->name, task, task->parent->name, task->parent);
 		uring_task_get(NULL, task->parent);
+	}
 }
 
 void
@@ -432,7 +465,7 @@ uring_poll_cancel(struct cfg *cfg, struct uring_task *task)
 	struct io_uring_sqe *sqe;
 	
 	if (task->fd < 0) {
-		error("uring_poll_cancel called with no fd set\n");
+		fprintf(stderr, "uring_poll_cancel called with no fd set\n");
 		return;
 	}
 
@@ -442,16 +475,6 @@ uring_poll_cancel(struct cfg *cfg, struct uring_task *task)
 	io_uring_sqe_set_data(sqe, NULL);
 }
 
-void
-uring_cancel(struct cfg *cfg, struct uring_task *task)
-{
-	struct io_uring_sqe *sqe = get_sqe(cfg);
-	
-	task->dead = true;
-	io_uring_prep_cancel(sqe, task, 0);
-	io_uring_sqe_set_data(sqe, NULL);
-}
-
 static void
 uring_free(struct uring_task *task)
 {
@@ -470,6 +493,12 @@ uring_refdump(struct uring_ev *uev)
 	uring_task_refdump(&uev->task);
 }
 
+void
+uring_delete(struct cfg *cfg)
+{
+	uring_task_put(cfg, &cfg->uev->task);
+}
+
 void
 uring_init(struct cfg *cfg)
 {
diff --git a/uring.h b/uring.h
index d044f05..1ceb282 100644
--- a/uring.h
+++ b/uring.h
@@ -3,6 +3,8 @@
 
 void uring_task_refdump(struct uring_task *task);
 
+void uring_task_destroy(struct cfg *cfg, struct uring_task *task);
+
 void uring_task_put(struct cfg *cfg, struct uring_task *task);
 
 void uring_task_get(struct cfg *cfg, struct uring_task *task);
@@ -61,7 +63,7 @@ void uring_poll(struct cfg *cfg, struct uring_task *task, short poll_mask,
 
 void uring_poll_cancel(struct cfg *cfg, struct uring_task *task);
 
-void uring_cancel(struct cfg *cfg, struct uring_task *task);
+void uring_delete(struct cfg *cfg);
 
 void uring_refdump(struct uring_ev *uev);
 
-- 
cgit v1.2.3