Safely restart services to avoid race conditions.

At present, service restarts exhibit a race condition whereby the new
(restarting) service process is often spawned before the old (stopping)
process has terminated.  This may result in the new service process failing
to acquire a limited resource (file lock, socket bind, etc.) that the old
process has not yet released.

The new method preforms a safe service restart by stopping the service,
waiting for the old service process to terminate, and (only then) start the
new service process.  In the event of "restarting" an already stopped
service, the previous behavior is maintained whereby the service is simply
started.

4 files changed, 32 insertions, 9 deletions

diff --git a/init/builtins.c b/init/builtins.c
index f16f07d6..87d19141 100644
--- a/init/builtins.c
+++ b/init/builtins.c
@@ -641,8 +641,7 @@ int do_restart(int nargs, char **args)
     struct service *svc;
     svc = service_find_by_name(args[1]);
     if (svc) {
-        service_stop(svc);
-        service_start(svc, NULL);
+        service_restart(svc);
     }
     return 0;
 }
diff --git a/init/init.c b/init/init.c
index 576d305c..5df254c5 100755
--- a/init/init.c
+++ b/init/init.c
@@ -172,7 +172,7 @@ void service_start(struct service *svc, const char *dynamic_args)
          * state and immediately takes it out of the restarting
          * state if it was in there
          */
-    svc->flags &= (~(SVC_DISABLED|SVC_RESTARTING|SVC_RESET));
+    svc->flags &= (~(SVC_DISABLED|SVC_RESTARTING|SVC_RESET|SVC_RESTART));
     svc->time_started = 0;
 
         /* running processes require no additional work -- if
@@ -370,15 +370,15 @@ void service_start(struct service *svc, const char *dynamic_args)
         notify_service_state(svc->name, "running");
 }
 
-/* The how field should be either SVC_DISABLED or SVC_RESET */
+/* The how field should be either SVC_DISABLED, SVC_RESET, or SVC_RESTART */
 static void service_stop_or_reset(struct service *svc, int how)
 {
         /* we are no longer running, nor should we
-         * attempt to restart
+         * attempt to restart (yet)
          */
     svc->flags &= (~(SVC_RUNNING|SVC_RESTARTING));
 
-    if ((how != SVC_DISABLED) && (how != SVC_RESET)) {
+    if ((how != SVC_DISABLED) && (how != SVC_RESET) && (how != SVC_RESTART)) {
         /* Hrm, an illegal flag.  Default to SVC_DISABLED */
         how = SVC_DISABLED;
     }
@@ -410,6 +410,17 @@ void service_stop(struct service *svc)
     service_stop_or_reset(svc, SVC_DISABLED);
 }
 
+void service_restart(struct service *svc)
+{
+    if (svc->flags & SVC_RUNNING) {
+        /* Stop, wait, then start the service. */
+        service_stop_or_reset(svc, SVC_RESTART);
+    } else if (!(svc->flags & SVC_RESTARTING)) {
+        /* Just start the service since it's not running. */
+        service_start(svc, NULL);
+    } /* else: Service is restarting anyways. */
+}
+
 void property_changed(const char *name, const char *value)
 {
     if (property_triggers_enabled)
@@ -476,6 +487,17 @@ static void msg_stop(const char *name)
     }
 }
 
+static void msg_restart(const char *name)
+{
+    struct service *svc = service_find_by_name(name);
+
+    if (svc) {
+        service_restart(svc);
+    } else {
+        ERROR("no such service '%s'\n", name);
+    }
+}
+
 void handle_control_message(const char *msg, const char *arg)
 {
     if (!strcmp(msg,"start")) {
@@ -483,8 +505,7 @@ void handle_control_message(const char *msg, const char *arg)
     } else if (!strcmp(msg,"stop")) {
         msg_stop(arg);
     } else if (!strcmp(msg,"restart")) {
-        msg_stop(arg);
-        msg_start(arg);
+        msg_restart(arg);
     } else {
         ERROR("unknown control msg '%s'\n", msg);
     }
diff --git a/init/init.h b/init/init.h
index 58bbbfe9..c20864ad 100644
--- a/init/init.h
+++ b/init/init.h
@@ -72,6 +72,7 @@ struct svcenvinfo {
 #define SVC_RESET       0x40  /* Use when stopping a process, but not disabling
                                  so it can be restarted with its class */
 #define SVC_RC_DISABLED 0x80  /* Remember if the disabled flag was set in the rc script */
+#define SVC_RESTART     0x100 /* Use to safely restart (stop, wait, start) a service */
 
 #define NR_SVC_SUPP_GIDS 12    /* twelve supplementary groups */
 
@@ -129,6 +130,7 @@ void service_for_each_flags(unsigned matchflags,
                             void (*func)(struct service *svc));
 void service_stop(struct service *svc);
 void service_reset(struct service *svc);
+void service_restart(struct service *svc);
 void service_start(struct service *svc, const char *dynamic_args);
 void property_changed(const char *name, const char *value);
 
diff --git a/init/signal_handler.c b/init/signal_handler.c
index b1701325..7efd3f14 100644
--- a/init/signal_handler.c
+++ b/init/signal_handler.c
@@ -90,7 +90,7 @@ static int wait_for_one_process(int block)
     }
 
     now = gettime();
-    if (svc->flags & SVC_CRITICAL) {
+    if ((svc->flags & SVC_CRITICAL) && !(svc->flags & SVC_RESTART)) {
         if (svc->time_crashed + CRITICAL_CRASH_WINDOW >= now) {
             if (++svc->nr_crashed > CRITICAL_CRASH_THRESHOLD) {
                 ERROR("critical process '%s' exited %d times in %d minutes; "
@@ -105,6 +105,7 @@ static int wait_for_one_process(int block)
         }
     }
 
+    svc->flags &= (~SVC_RESTART);
     svc->flags |= SVC_RESTARTING;
 
     /* Execute all onrestart commands for this service. */