From cb9f6213dc1b0334c18802b65164fe6a4f2c79fc Mon Sep 17 00:00:00 2001
From: Joachim Nilsson <troglobit@gmail.com>
Date: Tue, 2 Oct 2018 13:22:15 +0200
Subject: [PATCH] Instead of deleting svc_t, mark it for cleanup by garbage
 collector

When a process, that has been sent SIGTERM by us, takes a full 3 sec to
terminate, the event loop may have both the SIGCHLD event (where we do
the svc_del()) and the 3 sec timeout event to send SIGKILL in its event
cache.

When we call svc_del() it releases the svc_t memory, which can then be
dereferenced by the SIGKILL timer callback and we're doomed.

There are two fixes to this; 1) the event loop, that uses epoll_wait(),
can set maxevents=1 (instead of today's 10).  The kernel will then drop
the SIGKILL timer event before it's delivered to the userspace process.
2) we can postpone deleting the svc_t to a "later stage" when all events
in the event cache have been processed.

This patch implements (2).  A later patch will use uev_init1(ctx, 1) to
ensure the event cache handles only one event at a time.

Signed-off-by: Joachim Nilsson <troglobit@gmail.com>
---
 src/svc.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++----
 src/svc.h |  3 +++
 2 files changed, 51 insertions(+), 4 deletions(-)

diff --git a/src/svc.c b/src/svc.c
index a80fd810..8e268081 100644
--- a/src/svc.c
+++ b/src/svc.c
@@ -24,6 +24,7 @@
 
 #include <err.h>
 #include <ctype.h>		/* isdigit() */
+#include <time.h>
 #include <stdlib.h>
 #include <strings.h>
 #include <sys/time.h>
@@ -37,7 +38,48 @@
 
 /* Each svc_t needs a unique job# */
 static int jobcounter = 1;
-static TAILQ_HEAD(head, svc) svc_list = TAILQ_HEAD_INITIALIZER(svc_list);
+static TAILQ_HEAD(, svc) svc_list = TAILQ_HEAD_INITIALIZER(svc_list);
+static TAILQ_HEAD(, svc) gc_list  = TAILQ_HEAD_INITIALIZER(gc_list);
+
+static uev_t gc_timer;
+static int   gc_init = 0;
+
+static void gc(uev_t *w, void *arg, int events)
+{
+	struct timespec now;
+	svc_t *svc, *next;
+
+	if (UEV_ERROR == events) {
+		uev_timer_start(w);
+		return;
+	}
+
+	clock_gettime(CLOCK_MONOTONIC_COARSE, &now);
+	TAILQ_FOREACH_SAFE(svc, &gc_list, link, next) {
+		int msec;
+
+		msec  = (now.tv_sec  - svc->gc.tv_sec)  * 1000 +
+			(now.tv_nsec - svc->gc.tv_nsec) / 1000000;
+		if (msec < SVC_TERM_TIMEOUT)
+			continue;
+
+		TAILQ_REMOVE(&gc_list, svc, link);
+		free(svc);
+	}
+
+	if (!TAILQ_EMPTY(&gc_list))
+		uev_timer_set(&gc_timer, SVC_TERM_TIMEOUT, 0);
+}
+
+static int schedule_gc(int msec)
+{
+	if (!gc_init) {
+		gc_init =1;
+		return uev_timer_init(ctx, &gc_timer, gc, NULL, msec, 0);
+	}
+
+	return uev_timer_set(&gc_timer, msec, 0);
+}
 
 /**
  * svc_new - Create a new service
@@ -87,7 +129,7 @@ svc_t *svc_new(char *cmd, int id, int type)
 }
 
 /**
- * svc_del - Delete a service object
+ * svc_del - Mark a service object for deletion
  * @svc: Pointer to an &svc_t object
  *
  * Returns:
@@ -96,8 +138,10 @@ svc_t *svc_new(char *cmd, int id, int type)
 int svc_del(svc_t *svc)
 {
 	TAILQ_REMOVE(&svc_list, svc, link);
-	memset(svc, 0, sizeof(*svc));
-	free(svc);
+	TAILQ_INSERT_TAIL(&gc_list, svc, link);
+
+	clock_gettime(CLOCK_MONOTONIC_COARSE, &svc->gc);
+	schedule_gc(SVC_TERM_TIMEOUT);
 
 	return 0;
 }
diff --git a/src/svc.h b/src/svc.h
index 3e14d4f2..4daf6c38 100644
--- a/src/svc.h
+++ b/src/svc.h
@@ -138,6 +138,9 @@ typedef struct svc {
 	 */
 	uev_t          timer;
 	void           (*timer_cb)(struct svc *svc);
+
+	/* time at svc_del(), used by gc timer */
+	struct timespec gc;
 } svc_t;
 
 svc_t      *svc_new                (char *cmd, int id, int type);