From fb99bdf64e74c20015ccc40b7339d85808a129cf Mon Sep 17 00:00:00 2001
From: Davin McCall <davmac@davmac.org>
Date: Mon, 8 Jan 2018 22:00:43 +0000
Subject: [PATCH] Properly handle scripted service start interrupt.

If we issue an interrupt (SIGINT) to a starting script, but the script
completes normally, we should run the stop script before marking the
service as stopped.
---
 src/service.cc | 42 +++++++++++++++++++++++++++++++++---------
 src/service.h  | 22 +++++++++++++++++-----
 2 files changed, 50 insertions(+), 14 deletions(-)

diff --git a/src/service.cc b/src/service.cc
index 1de7212..ddf49ca 100644
--- a/src/service.cc
+++ b/src/service.cc
@@ -338,21 +338,45 @@ void scripted_service::handle_exit_status(int exit_status) noexcept
     auto service_state = get_state();
 
     if (service_state == service_state_t::STOPPING) {
+        // We might be running the stop script, or we might be running the start script and have issued
+        // a cancel order via SIGINT:
         if (did_exit && WEXITSTATUS(exit_status) == 0) {
-            stopped();
+            if (interrupting_start) {
+                interrupting_start = false;
+                // launch stop script:
+                bring_down();
+            }
+            else {
+                // We were running the stop script and finished successfully
+                stopped();
+            }
         }
         else {
-            // ??? failed to stop! Let's log it as info:
-            if (did_exit) {
-                log(loglevel_t::INFO, "Service ", get_name(), " stop command failed with exit code ",
-                        WEXITSTATUS(exit_status));
+            if (interrupting_start) {
+                // We issued a start interrupt, so we expected this failure:
+                if (did_exit) {
+                    log(loglevel_t::INFO, "Service ", get_name(), " start cancelled; exit code ",
+                            WEXITSTATUS(exit_status));
+                }
+                else if (was_signalled) {
+                    log(loglevel_t::INFO, "Service ", get_name(), " start cancelled from signal ",
+                            WTERMSIG(exit_status));
+                }
             }
-            else if (was_signalled) {
-                log(loglevel_t::INFO, "Service ", get_name(), " stop command terminated due to signal ",
-                        WTERMSIG(exit_status));
+            else {
+                // ??? failed to stop! Let's log it as warning:
+                if (did_exit) {
+                    log(loglevel_t::WARN, "Service ", get_name(), " stop command failed with exit code ",
+                            WEXITSTATUS(exit_status));
+                }
+                else if (was_signalled) {
+                    log(loglevel_t::WARN, "Service ", get_name(), " stop command terminated due to signal ",
+                            WTERMSIG(exit_status));
+                }
             }
             // Just assume that we stopped, so that any dependencies
             // can be stopped:
+            interrupting_start = false;
             stopped();
         }
         services->process_queues();
@@ -1185,7 +1209,7 @@ void service_record::do_stop() noexcept
             }
 
             if (! interrupt_start()) {
-                // Now wait for service startup to actually end
+                // Now wait for service startup to actually end; we don't need to handle it here.
                 return;
             }
 
diff --git a/src/service.h b/src/service.h
index 2cde697..8e10a21 100644
--- a/src/service.h
+++ b/src/service.h
@@ -458,7 +458,7 @@ class service_record
     }
 
     // Interrupt startup. Returns true if service start is fully cancelled; returns false if cancel order
-    // issued but service has not yet responded.
+    // issued but service has not yet responded (state will be set to STOPPING).
     virtual bool interrupt_start() noexcept;
 
     public:
@@ -662,9 +662,6 @@ class base_process_service : public service_record
     bool tracking_child : 1;  // whether we expect to see child process status
     bool start_is_interruptible : 1;  // whether we can interrupt start
 
-    // Start the process, return true on success
-    virtual bool bring_up() noexcept override;
-
     // Launch the process with the given arguments, return true on success
     bool start_ps_process(const std::vector<const char *> &args, bool on_console) noexcept;
 
@@ -675,11 +672,16 @@ class base_process_service : public service_record
     // Perform smooth recovery process
     void do_smooth_recovery() noexcept;
 
+    // Start the process, return true on success
+    virtual bool bring_up() noexcept override;
+
     virtual void bring_down() noexcept override;
 
     // Called when the process exits. The exit_status is the status value yielded by
     // the "wait" system call.
     virtual void handle_exit_status(int exit_status) noexcept = 0;
+
+    // Called if an exec fails.
     virtual void exec_failed(int errcode) noexcept = 0;
 
     virtual bool can_interrupt_start() noexcept override
@@ -797,12 +799,22 @@ class scripted_service : public base_process_service
     virtual void exec_failed(int errcode) noexcept override;
     virtual void bring_down() noexcept override;
 
+    virtual bool interrupt_start() noexcept override
+    {
+        // if base::interrupt_start() returns false, then start hasn't been fully interrupted, but an
+        // interrupt has been issued:
+        interrupting_start = ! base_process_service::interrupt_start();
+        return ! interrupting_start;
+    }
+
+    bool interrupting_start : 1;  // running start script (true) or stop script (false)
+
     public:
     scripted_service(service_set *sset, string name, string &&command,
             std::list<std::pair<unsigned,unsigned>> &command_offsets,
             std::list<prelim_dep> depends_p)
          : base_process_service(sset, name, service_type_t::SCRIPTED, std::move(command), command_offsets,
-             depends_p)
+             depends_p), interrupting_start(false)
     {
     }
 
-- 
2.25.1