On reload/restart we must wait for services to stop first

This patch adds support for declaring *why* a service is being stopped
or reloaded.  This is later used when all stopped services have been
collected by the `service_monitor()` to issue start or postponed SIGHUP.

As an added benefit we can now also see why a service is not running.
If it has been paused by a user, halted when moving to another runlevel,
waiting for a condition (event), or similar.

Signed-off-by: Joachim Nilsson <troglobit@gmail.com>

README.md

@@ -597,7 +597,7 @@ not return until the given command has fully completed.
             reload          Reload *.conf in /etc/finit.d/, like SIGHUP
             runlevel <0-9>  Change runlevel: 0 (halt), 6 (reboot)
             start    <JOB#> Start a stopped service
-            stop     <JOB#> Stop a running service
+            stop     <JOB#> Stop/Pause a running service
             restart  <JOB#> Restart (stop/start) a running service
             reload   <JOB#> Reload (SIGHUP) a running service
             status | show   List status of all services known to Finit
@@ -610,6 +610,11 @@ their command counterparts.  The `reload` command reloads all `*.conf`
 files in `/etc/finit.d/` *and* activates the changes, with `emit RELOAD`
 only the `*.conf` files are reloaded.
 
+**Note:** The `emit STOP` event is more like "prepare" than "stop".
+  Depending on how the service is declared, `<!>` or not, the service
+  may be stopped, or skipped to be be sent `SIGHUP` later when `emit
+  START` is issued.
+
 To achieve the same result as `initctl reload` emit all three events:
 
 ```shell

api.c

@@ -108,8 +108,13 @@ static int call(int (*action)(svc_t *), char *buf, size_t len)
 	return result;
 }
 
+static int service_pause(svc_t *svc)
+{
+	return service_stop(svc, SVC_PAUSED_STATE);
+}
+
 static int do_start  (char *buf, size_t len) { return call(service_start,   buf, len); }
-static int do_stop   (char *buf, size_t len) { return call(service_stop,    buf, len); }
+static int do_pause  (char *buf, size_t len) { return call(service_pause,   buf, len); }
 static int do_reload (char *buf, size_t len) { return call(service_reload,  buf, len); }
 static int do_restart(char *buf, size_t len) { return call(service_restart, buf, len); }
 
@@ -258,7 +263,7 @@ static void cb(uev_t *w, void *UNUSED(arg), int UNUSED(events))
 			debug = !debug;
 			break;
 
-		case INIT_CMD_RELOAD:
+		case INIT_CMD_RELOAD: /* 'init q' and 'initctl reload' */
 			service_reload_dynamic();
 			break;
 
@@ -267,7 +272,7 @@ static void cb(uev_t *w, void *UNUSED(arg), int UNUSED(events))
 			break;
 
 		case INIT_CMD_STOP_SVC:
-			result = do_stop(rq.data, sizeof(rq.data));
+			result = do_pause(rq.data, sizeof(rq.data));
 			break;
 
 		case INIT_CMD_RELOAD_SVC:

event.c

@@ -220,12 +220,11 @@ void event_dispatch(char *msg)
 				service_restart(svc);
 		} else { /* change == -1 */
 			if (svc->pid && !event_service_cond(svc->events))
-				service_stop(svc);
+				service_stop(svc, SVC_CONDHALT_STATE);
 		}
 	}
 }
 
-
 /**
  * Local Variables:
  *  version-control: t

inetd.c

@@ -212,7 +212,7 @@ void inetd_stop(inetd_t *inetd)
 
 	/* Stop any running service, not allowed anymore. */
 	if (svc->pid)
-		service_stop(svc);
+		service_stop(svc, SVC_WAITING_STATE);
 }
 
 static int getent(char *service, char *proto, struct servent **sv, struct protoent **pv)

initctl.c

@@ -218,7 +218,7 @@ static int usage(int rc)
 		"  runlevel [0-9]        Show or set runlevel: 0 halt, 6 reboot\n"
 		"  status | show         Show status of services\n"
 		"  start    <JOB|NAME>   Start stopped job or service\n"
-		"  stop     <JOB|NAME>   Stop running job or service\n"
+		"  stop     <JOB|NAME>   Stop/Pause running job or service\n"
 		"  restart  <JOB|NAME>   Restart (stop/start) job or service\n"
 		"  reload   <JOB|NAME>   Reload (SIGHUP) job or service\n"
 		"  version               Show Finit version\n\n", __progname);

service.c

@@ -90,6 +90,8 @@ void service_bootstrap(void)
  */
 svc_cmd_t service_enabled(svc_t *svc, int event, void *arg)
 {
+	svc_cmd_t cmd = SVC_START; /* Default to start, since listed in finit.conf */
+
 	if (!svc) {
 		errno = EINVAL;
 		return SVC_STOP;
@@ -104,6 +106,11 @@ svc_cmd_t service_enabled(svc_t *svc, int event, void *arg)
 	if (runlevel && !event_service_cond(svc->events))
 		return SVC_STOP;
 
+	if (svc->state == SVC_RELOAD_STATE)
+		cmd = SVC_RELOAD;
+	if (svc->state == SVC_PAUSED_STATE)
+		cmd = SVC_STOP;
+
 	/* Is there a service plugin registered? */
 	if (svc->cb) {
 		int   status;
@@ -125,8 +132,10 @@ svc_cmd_t service_enabled(svc_t *svc, int event, void *arg)
 		}
 
 		/* Callback normally exits here. */
-		if (WIFEXITED(status))
-			return WEXITSTATUS(status);
+		if (WIFEXITED(status)) {
+			svc_cmd_t tmp = WEXITSTATUS(status);
+			return tmp == SVC_START ? cmd : tmp;
+		}
 
 		/* Check for SEGFAULT or other error ... */
 		if (WIFSIGNALED(status) && WCOREDUMP(status))
@@ -137,8 +146,7 @@ svc_cmd_t service_enabled(svc_t *svc, int event, void *arg)
 		return SVC_STOP;
 	}
 
-	/* No service plugin, default to start, since listed in finit.conf */
-	return SVC_START;
+	return cmd;
 }
 
 /* Remember: service_enabled() must be called before calling service_start() */
@@ -297,6 +305,7 @@ int service_start(svc_t *svc)
 		exit(status);
 	}
 	svc->pid = pid;
+	svc->state = SVC_RUNNING_STATE;
 
 	if (svc_is_inetd(svc)) {
 		if (svc->inetd.type == SOCK_STREAM)
@@ -318,9 +327,9 @@ int service_start(svc_t *svc)
 	return 0;
 }
 
-int service_stop(svc_t *svc)
+int service_stop(svc_t *svc, int state)
 {
-	int res = 1;
+	int res = 0;
 
 	if (!svc)
 		return 1;
@@ -334,6 +343,12 @@ int service_stop(svc_t *svc)
 	if (SVC_TYPE_SERVICE != svc->type)
 		return 0;
 
+	_d("Service %s state %d, new state %d, sighup %d", svc->cmd, svc->state, state, svc->sighup);
+	if (state == SVC_RELOAD_STATE && svc->sighup)
+		goto exit;
+	if (state == SVC_RESTART_STATE && svc->sighup)
+		return service_reload(svc);
+
 	if (runlevel != 1 && verbose)
 		print_desc("Stopping ", svc->desc);
 
@@ -343,7 +358,8 @@ int service_stop(svc_t *svc)
 	if (runlevel != 1 && verbose)
 		print_result(res);
 exit:
-	svc->pid = 0;
+	if (!res)
+		svc->state = state;
 	svc->restart_counter = 0;
 
 	return res;
@@ -381,6 +397,10 @@ static int service_stop_done(svc_t *svc)
 	if (!dyn_stop_cnt) {
 		_d("All disabled/removed services have been stoppped, calling reconf hooks ...");
 		plugin_run_hooks(HOOK_SVC_RECONF); /* Reconfigure HW/VLANs/etc here */
+
+		/* Finish off by starting/reload modified/new services */
+		if (svc->state == SVC_RELOAD_STATE)
+			service_start_dynamic();
 	}
 
 	return 1;
@@ -388,6 +408,10 @@ static int service_stop_done(svc_t *svc)
 
 /**
  * service_stop_dynamic - Stop disabled/removed dynamic services
+ *
+ * We call it "stop", but in reality it could be "skip" as well, if the
+ * service supports SIGHUP.  This function is just one step on the road
+ * to reload all modified services.
  */
 void service_stop_dynamic(void)
 {
@@ -397,7 +421,8 @@ void service_stop_dynamic(void)
 	for (svc = svc_dynamic_iterator(1); svc; svc = svc_dynamic_iterator(0)) {
 		if (svc_is_changed(svc)) {
 			dyn_stop_cnt++;
-			service_stop(svc);
+			_d("Marking service %s as state %d", svc->cmd, SVC_RELOAD_STATE);
+			service_stop(svc, SVC_RELOAD_STATE);
 		}
 	}
 
@@ -405,23 +430,30 @@ void service_stop_dynamic(void)
 	service_stop_done(NULL);
 }
 
-/* stop + start
+/**
+ * service_restart - Restart or SIGHUP a process
  *
- * XXX: This should be refactored into a background job AND marked such
- *      so that the service monitor doesn't attempt to restart it!
+ * Unfortunately we cannot call service_start() here because we must
+ * wait for the svc to actually be collected by the service_monitor()
+ * first.  Currently this means that an initctl user will not get
+ * correct feedback when restarting a service.  (Prompt may return
+ * before we've actually started upp the service again.)
+ *
+ * XXX: This should be refactored to let initctl check the sighup
+ *      flag and then to stop+start or reload.
  */
 int service_restart(svc_t *svc)
 {
-	int result = 0;
-
 	if (!svc)
 		return 1;
 
-	result += service_stop(svc);
-	sleep(1);
-	result += service_start(svc);
+	/* Ignore if finit is SIGSTOP'ed */
+	if (is_norespawn())
+		return 0;
 
-	return result;
+	svc->restart_counter = 0;
+
+	return service_stop(svc, SVC_RESTART_STATE);
 }
 
 /**
@@ -440,16 +472,15 @@ int service_reload(svc_t *svc)
 	if (is_norespawn())
 		return 0;
 
-	if (!svc) {
-		_e("Failed, no svc pointer.");
+	if (!svc)
 		return 1;
-	}
 
 	if (svc->pid <= 1) {
 		_d("Bad PID %d for %s, SIGHUP", svc->pid, svc->cmd);
 		svc->pid = 0;
 		return 1;
 	}
+	svc->state = SVC_RUNNING_STATE;
 
 	_d("Sending SIGHUP to PID %d", svc->pid);
 	return kill(svc->pid, SIGHUP);
@@ -470,8 +501,12 @@ void service_reload_dynamic(void)
 	/* Then stop any disabled/removed services and non-reloadable */
 	service_stop_dynamic();
 
-	/* Finish off by starting/reload modified/new services */
-	service_start_dynamic();
+	/*
+	 * Finish off by starting/reloading modified/new services.
+	 * Postponed to service_stop_done() to make sure all services
+	 * are guaranteed to have been stopped before being started
+	 * again.
+	 */
 }
 
 /**
@@ -508,12 +543,12 @@ void service_runlevel(int newlevel)
 				inetd_stop(&svc->inetd);
 			else
 #endif
-				service_stop(svc);
+				service_stop(svc, SVC_HALTED_STATE);
 		}
 
 		/* ... or disabled/removed services from /etc/finit.d/ */
 		if (svc_is_dynamic(svc) && svc_is_changed(svc))
-			service_stop(svc);
+			service_stop(svc, SVC_HALTED_STATE);
 	}
 
 	/* Prev runlevel services stopped, call hooks before starting new runlevel ... */
@@ -787,7 +822,7 @@ int service_register(int type, char *line, time_t mtime, char *username)
 
 void service_unregister(svc_t *svc)
 {
-	service_stop(svc);
+	service_stop(svc, SVC_HALTED_STATE);
 	svc_del(svc);
 }
 
@@ -890,7 +925,7 @@ static void svc_dance(svc_t *svc)
 
 	if (svc->pid) {
 		if (SVC_STOP == cmd)
-			service_stop(svc);
+			service_stop(svc, SVC_HALTED_STATE);
 		else if (SVC_RELOAD == cmd)
 			service_reload(svc);
 	} else {

service.h

@@ -33,7 +33,7 @@ void      service_unregister     (svc_t *svc);
 svc_cmd_t service_enabled	 (svc_t *svc, int event, void *arg);
 
 int	  service_start	         (svc_t *svc);
-int	  service_stop	         (svc_t *svc);
+int	  service_stop	         (svc_t *svc, int state);
 void      service_start_dynamic  (void);
 void      service_stop_dynamic   (void);
 int       service_restart        (svc_t *svc);

sig.c

@@ -96,6 +96,7 @@ void do_shutdown(int sig)
  */
 static void sighup_cb(uev_t *UNUSED(w), void *UNUSED(arg), int UNUSED(events))
 {
+	/* INIT_CMD_RELOAD: 'init q', 'initctl reload', and SIGHUP */
 	service_reload_dynamic();
 }
 

svc.h

@@ -40,12 +40,22 @@ typedef enum {
 
 typedef enum {
 	SVC_TYPE_FREE = 0,	/* Free to allocate */
-	SVC_TYPE_SERVICE ,	/* Monitored, will be respawned */
+	SVC_TYPE_SERVICE,	/* Monitored, will be respawned */
 	SVC_TYPE_TASK,		/* One-shot, runs in parallell */
 	SVC_TYPE_RUN,		/* Like task, but wait for completion */
 	SVC_TYPE_INETD		/* Classic inetd service */
 } svc_type_t;
 
+typedef enum {
+	SVC_HALTED_STATE = 0,	/* Not allowed in runlevel, or not enabled. */
+	SVC_WAITING_STATE,	/* Waiting for connection (inetd service)   */
+	SVC_PAUSED_STATE,	/* Stopped/Paused by user started on reload */
+	SVC_CONDHALT_STATE,	/* Not allowed to run atm. event/state lost */
+	SVC_RESTART_STATE,	/* Restarting service waiting to be stopped */
+	SVC_RELOAD_STATE,	/* Reloading services, after .conf changed  */
+	SVC_RUNNING_STATE,	/* Currently running service, see svc->pid  */
+} svc_state_t;
+
 #define FINIT_SHM_ID     0x494E4954  /* "INIT", see ascii(7) */
 #define MAX_ARG_LEN      64
 #define MAX_STR_LEN      64
@@ -63,6 +73,7 @@ typedef struct svc {
 
 	/* Service details */
 	pid_t	       pid;
+	svc_state_t    state;	       /* Paused, Reloading, Restart, Running, ... */
 	svc_type_t     type;
 	time_t	       mtime;	       /* Modification time for .conf from /etc/finit.d/ */
 	int            dirty;	       /* Set if old mtime != new mtime  => reloaded,