Wait for post:script and cleanup:script before returning OK

This change introduces two new states for the big Finit state machine: runlevel-clean and reload-clean. Here Finit now waits for any post or cleanup script to finish before returning OK to the initctl command. Additionally, the service state machine has been updated to ensure a run/task/sysv/service calls any post or cleanup script before they are removed. A new 'dead' state for svc_t is introduced which any removed svc_t ends up in now instead of becing collected from 'halted' state. Signed-off-by: Joachim Wiberg <[email protected]>
troglobit · Feb 22, 2025 · c17c0c9 · c17c0c9
1 parent 8db6b17
commit c17c0c9
Show file tree

Hide file tree

Showing 7 changed files with 121 additions and 23 deletions.
diff --git a/doc/svc-machine.png b/doc/svc-machine.png
diff --git a/doc/svc-machine.svg b/doc/svc-machine.svg
diff --git a/src/service.c b/src/service.c
@@ -2483,16 +2483,13 @@ int service_step(svc_t *svc)
 	   condstr(cond_get_agg(svc->cond)));
 
 	switch (svc->state) {
-	case SVC_CLEANUP_STATE:
-		if (!svc->pid)
-			svc_set_state(svc, SVC_HALTED_STATE);
-		break;
-
 	case SVC_HALTED_STATE:
 		if (enabled)
 			svc_set_state(svc, SVC_WAITING_STATE);
 		else {
-			if (svc_is_conflict(svc)) {
+			if (svc_is_removed(svc)) {
+				svc_set_state(svc, SVC_DEAD_STATE);
+			} else if (svc_is_conflict(svc)) {
 #if 0
 				logit(svc->nowarn ? LOG_DEBUG : LOG_INFO,
 				      "%s in conflict with %s, checking again ...",
@@ -2504,6 +2501,30 @@ int service_step(svc_t *svc)
 		}
 		break;
 
+	case SVC_TEARDOWN_STATE:
+		if (!svc->pid) {
+			dbg("%s: post script done.", svc_ident(svc, NULL, 0));
+			service_timeout_cancel(svc);
+
+			if (svc_is_removed(svc) && svc_has_cleanup(svc)) {
+				svc_set_state(svc, SVC_CLEANUP_STATE);
+				service_cleanup_script(svc);
+			} else
+				svc_set_state(svc, SVC_HALTED_STATE);
+		}
+		break;
+
+	case SVC_CLEANUP_STATE:
+		if (!svc->pid) {
+			dbg("%s: cleanup script done.", svc_ident(svc, NULL, 0));
+			svc_set_state(svc, SVC_HALTED_STATE);
+		}
+		break;
+
+	case SVC_DEAD_STATE:
+		/* End of the line ☠ */
+		break;
+
 	case SVC_DONE_STATE:
 		if (svc_is_changed(svc))
 			svc_set_state(svc, SVC_HALTED_STATE);
@@ -2528,6 +2549,9 @@ int service_step(svc_t *svc)
 				if (svc_has_post(svc)) {
 					svc_set_state(svc, SVC_TEARDOWN_STATE);
 					service_post_script(svc);
+				} else if (svc_is_removed(svc) && svc_has_cleanup(svc)) {
+					svc_set_state(svc, SVC_CLEANUP_STATE);
+					service_cleanup_script(svc);
 				} else
 					svc_set_state(svc, SVC_HALTED_STATE);
 				break;
@@ -2546,16 +2570,6 @@ int service_step(svc_t *svc)
 		}
 		break;
 
-	case SVC_TEARDOWN_STATE:
-		if (!svc->pid) {
-			if (svc_is_removed(svc) && svc_has_cleanup(svc)) {
-				svc_set_state(svc, SVC_CLEANUP_STATE);
-				service_cleanup_script(svc);
-			} else
-				svc_set_state(svc, SVC_HALTED_STATE);
-		}
-		break;
-
 	case SVC_SETUP_STATE:
 		if (!svc->pid) {
 			int rc = WEXITSTATUS(svc->status);

diff --git a/src/sm.c b/src/sm.c
@@ -107,6 +107,9 @@ static char *sm_status(sm_state_t state)
 	case SM_BOOTSTRAP_STATE:
 		return "bootstrap";
 
+	case SM_BOOTSTRAP_WAIT_STATE:
+		return "bootstrap/wait";
+
 	case SM_RUNNING_STATE:
 		return "running";
 
@@ -116,15 +119,20 @@ static char *sm_status(sm_state_t state)
 	case SM_RUNLEVEL_WAIT_STATE:
 		return "runlevel/wait";
 
+	case SM_RUNLEVEL_CLEAN_STATE:
+		return "runlevel/clean";
+
 	case SM_RELOAD_CHANGE_STATE:
 		return "reload/change";
 
 	case SM_RELOAD_WAIT_STATE:
 		return "reload/wait";
 
-	default:
-		return "unknown";
+	case SM_RELOAD_CLEAN_STATE:
+		return "reload/clean";
 	}
+
+	return "unknown";
 }
 
 static char sm_runlevel(int lvl)
@@ -319,6 +327,22 @@ void sm_step(sm_t *sm)
 		sm->in_teardown = 0;
 		service_step_all(SVC_TYPE_ANY);
 
+		sm->state = SM_RUNLEVEL_CLEAN_STATE;
+		break;
+
+	case SM_RUNLEVEL_CLEAN_STATE:
+		/*
+		 * Wait for post:script or cleanup:script to be collected,
+		 * which moves the svc to HALTED or DEAD state.  We will
+		 * be called by the service_monitor() on collect.
+		 */
+		svc = svc_clean_completed();
+		if (svc) {
+			dbg("Waiting to collect post/cleanup script for %s, cmd %s(%d) ...",
+			    svc_ident(svc, NULL, 0), svc->cmd, svc->pid);
+			break;
+		}
+
 		/* Cleanup stale services */
 		svc_clean_dynamic(service_unregister);
 
@@ -372,6 +396,22 @@ void sm_step(sm_t *sm)
 		dbg("Starting services after reconf ...");
 		service_step_all(SVC_TYPE_ANY);
 
+		sm->state = SM_RELOAD_CLEAN_STATE;
+		break;
+
+	case SM_RELOAD_CLEAN_STATE:
+		/*
+		 * Wait for post:script or cleanup:script to be collected,
+		 * which moves the svc to HALTED or DEAD state.  We will
+		 * be called by the service_monitor() on collect.
+		 */
+		svc = svc_clean_completed();
+		if (svc) {
+			dbg("Waiting to collect post/cleanup script for %s, cmd %s(%d) ...",
+			    svc_ident(svc, NULL, 0), svc->cmd, svc->pid);
+			break;
+		}
+
 		/* Cleanup stale services */
 		svc_clean_dynamic(service_unregister);
 

diff --git a/src/sm.h b/src/sm.h
@@ -29,9 +29,11 @@ typedef enum {
 	SM_BOOTSTRAP_WAIT_STATE,  /* Waiting for bootstrap to complete */
 	SM_RUNNING_STATE,         /* Normal state, services running */
 	SM_RUNLEVEL_CHANGE_STATE, /* A runlevel change has occurred */
-	SM_RUNLEVEL_WAIT_STATE,   /* Waiting for all stopped runlevel processes to be halted */
+	SM_RUNLEVEL_WAIT_STATE,   /* Waiting for all stopped processes to halt */
+	SM_RUNLEVEL_CLEAN_STATE,  /* Wait for post:scripts and cleanup:scripts */
 	SM_RELOAD_CHANGE_STATE,   /* A reload event has occurred */
-	SM_RELOAD_WAIT_STATE,     /* Waiting for all stopped reload processes to be halted */
+	SM_RELOAD_WAIT_STATE,     /* Waiting for all stopped processes to halt */
+	SM_RELOAD_CLEAN_STATE,    /* Wait for post:scripts and cleanup:scripts */
 } sm_state_t;
 
 typedef struct sm {

diff --git a/src/svc.c b/src/svc.c
@@ -348,6 +348,43 @@ svc_t *svc_stop_completed(void)
 	return NULL;
 }
 
+
+/**
+ * svc_clean_completed - Have post: and cleanup: scripts finished?
+ *
+ * Called late in runlevel and reload transitions to check if all post: and
+ * cleanup:scripts have completed.  A removed service should end up in the
+ * DEAD state, while a service with a post:script ends up in HALTED.
+ *
+ * Returns:
+ * %NULL if post: and cleanup: scripts have run, otherwise a pointer to
+ * the first found svc_t waiting for post: or cleanup:script.
+ */
+svc_t *svc_clean_completed(void)
+{
+	svc_t *svc, *iter = NULL;
+
+	for (svc = svc_iterator(&iter, 1); svc; svc = svc_iterator(&iter, 0)) {
+		if (svc_enabled(svc))
+			continue;
+
+		if (svc_is_runtask(svc) && svc->state == SVC_DONE_STATE)
+			continue;
+
+		if (svc_is_removed(svc)) {
+			if (svc->state == SVC_DEAD_STATE)
+				continue; /* any cleanup:script done */
+		} else {
+			if (svc->state == SVC_HALTED_STATE)
+				continue; /* any post:script done */
+		}
+
+		return svc;
+	}
+
+	return NULL;
+}
+
 /**
  * svc_find - Find a service object by its full path name
  * @name: Full path name, e.g., /sbin/syslogd
@@ -580,7 +617,7 @@ void svc_clean_dynamic(void (*cb)(svc_t *))
 	svc_t *svc, *iter = NULL;
 
 	for (svc = svc_iterator(&iter, 1); svc; svc = svc_iterator(&iter, 0)) {
-		if (svc->removed && cb)
+		if (svc_is_removed(svc) && cb)
 			cb(svc);
 	}
 }

diff --git a/src/svc.h b/src/svc.h
@@ -58,6 +58,7 @@ typedef enum {
 typedef enum {
 	SVC_HALTED_STATE = 0,	/* Not allowed in runlevel, or not enabled. */
 	SVC_DONE_STATE,		/* Task/Run job has been run */
+	SVC_DEAD_STATE,		/* Process is dead and scheduled for removal */
 	SVC_CLEANUP_STATE,	/* Running cleanup: script */
 	SVC_TEARDOWN_STATE,	/* Running post: script */
 	SVC_STOPPING_STATE,	/* Waiting to collect the child process */
@@ -247,6 +248,7 @@ void	    svc_foreach	           (int (*cb)(svc_t *));
 void        svc_foreach_type       (int types, int (*cb)(svc_t *));
 
 svc_t	   *svc_stop_completed	   (void);
+svc_t      *svc_clean_completed    (void);
 
 void	    svc_mark		   (svc_t *svc);
 void	    svc_mark_dynamic       (void);
@@ -419,6 +421,9 @@ static inline char *svc_status(svc_t *svc)
 	case SVC_RUNNING_STATE:
 		return "running";
 
+	case SVC_DEAD_STATE:
+		return "dead";
+
 	default:
 		return "UNKNOWN";
 	}