Re-work console acquisition/release.

Use variables to track when a service is waiting for the console or has
acquired console, rather than using complex combinations of service
state.

diff --git a/src/baseproc-service.cc b/src/baseproc-service.cc
index 3d0d01fb4b72fbc66a189e5010b82d85861b511d..87f6088ef69182d873c22783b62407e423d5fbd6 100644 (file)
--- a/src/baseproc-service.cc
+++ b/src/baseproc-service.cc
@@ -149,7 +149,6 @@ bool base_process_service::start_ps_process(const std::vector<const char *> &cmd
 
 void base_process_service::bring_down() noexcept
 {
-    waiting_for_deps = false;
     if (pid != -1) {
         // The process is still kicking on - must actually kill it. We signal the process
         // group (-pid) rather than just the process as there's less risk then of creating
@@ -211,11 +210,6 @@ void base_process_service::do_restart() noexcept
     restart_interval_count++;
     auto service_state = get_state();
 
-    // We may be STARTING (regular restart) or STARTED ("smooth recovery"). This affects whether
-    // the process should be granted access to the console:
-    bool on_console = service_state == service_state_t::STARTING
-            ? onstart_flags.starts_on_console : onstart_flags.runs_on_console;
-
     if (service_state == service_state_t::STARTING) {
         // for a smooth recovery, we want to check dependencies are available before actually
         // starting:
@@ -225,7 +219,7 @@ void base_process_service::do_restart() noexcept
         }
     }
 
-    if (! start_ps_process(exec_arg_parts, on_console)) {
+    if (! start_ps_process(exec_arg_parts, have_console)) {
         restarting = false;
         if (service_state == service_state_t::STARTING) {
             failed_to_start();

diff --git a/src/includes/service.h b/src/includes/service.h
index a11262f14710526104a7ba9a78f864c67e110af4..4e9aafa3cdbabcf35ce18de94655b2841e76e4bb 100644 (file)
--- a/src/includes/service.h
+++ b/src/includes/service.h
@@ -267,6 +267,8 @@ class service_record
     bool pinned_started : 1;
     bool waiting_for_deps : 1;  // if STARTING, whether we are waiting for dependencies (inc console) to start
                                 // if STOPPING, whether we are waiting for dependents to stop
+    bool waiting_for_console : 1;   // waiting for exclusive console access (while STARTING)
+    bool have_console : 1;      // whether we have exclusive console access (STARTING/STARTED)
     bool waiting_for_execstat : 1;  // if we are waiting for exec status after fork()
     bool start_explicit : 1;    // whether we are are explicitly required to be started
 
@@ -346,7 +348,7 @@ class service_record
     // A dependency has reached STARTED state
     void dependency_started() noexcept;
     
-    void all_deps_started(bool haveConsole = false) noexcept;
+    void all_deps_started() noexcept;
 
     // Open the activation socket, return false on failure
     bool open_socket() noexcept;
@@ -446,8 +448,8 @@ class service_record
         : service_state(service_state_t::STOPPED), desired_state(service_state_t::STOPPED),
             auto_restart(false), smooth_recovery(false),
             pinned_stopped(false), pinned_started(false), waiting_for_deps(false),
-            waiting_for_execstat(false), start_explicit(false),
-            prop_require(false), prop_release(false), prop_failure(false),
+            waiting_for_console(false), have_console(false), waiting_for_execstat(false),
+            start_explicit(false), prop_require(false), prop_release(false), prop_failure(false),
             prop_start(false), prop_stop(false), restarting(false), force_stop(false)
     {
         services = set;

diff --git a/src/service.cc b/src/service.cc
index f5e3575c6fc3d6a2e42892d158b30f648465d071..e5351edee6a2fe2c7ba3cb904fefca4fd7004a9f 100644 (file)
--- a/src/service.cc
+++ b/src/service.cc
@@ -243,12 +243,12 @@ void service_record::execute_transition() noexcept
     if (service_state == service_state_t::STARTING || (service_state == service_state_t::STARTED
             && restarting)) {
         if (check_deps_started()) {
-            bool have_console = service_state == service_state_t::STARTED && onstart_flags.runs_on_console;
-            all_deps_started(have_console);
+            all_deps_started();
         }
     }
     else if (service_state == service_state_t::STOPPING) {
         if (stop_check_dependents()) {
+            waiting_for_deps = false;
             bring_down();
         }
     }
@@ -390,10 +390,9 @@ bool service_record::open_socket() noexcept
     return true;
 }
 
-void service_record::all_deps_started(bool has_console) noexcept
+void service_record::all_deps_started() noexcept
 {
-    if (onstart_flags.starts_on_console && ! has_console) {
-        waiting_for_deps = true;
+    if (onstart_flags.starts_on_console && ! have_console) {
         queue_for_console();
         return;
     }
@@ -417,12 +416,15 @@ void service_record::all_deps_started(bool has_console) noexcept
 
 void service_record::acquired_console() noexcept
 {
+    waiting_for_console = false;
+    have_console = true;
+
     if (service_state != service_state_t::STARTING) {
         // We got the console but no longer want it.
         release_console();
     }
     else if (check_deps_started()) {
-        all_deps_started(true);
+        all_deps_started();
     }
     else {
         // We got the console but can't use it yet.
@@ -433,7 +435,8 @@ void service_record::acquired_console() noexcept
 
 void service_record::started() noexcept
 {
-    if (onstart_flags.starts_on_console && ! onstart_flags.runs_on_console) {
+    // If we start on console but don't keep it, release it now:
+    if (have_console && ! onstart_flags.runs_on_console) {
         tcsetpgrp(0, getpgrp());
         release_console();
     }
@@ -464,10 +467,14 @@ void service_record::started() noexcept
 
 void service_record::failed_to_start(bool depfailed) noexcept
 {
-    if (!depfailed && onstart_flags.starts_on_console) {
+    if (have_console) {
         tcsetpgrp(0, getpgrp());
         release_console();
     }
+    if (waiting_for_console) {
+        services->unqueue_console(this);
+        waiting_for_console = false;
+    }
     
     log_service_failed(get_name());
     service_state = service_state_t::STOPPED;
@@ -549,7 +556,9 @@ void service_record::do_stop() noexcept
 
     if (service_state != service_state_t::STARTED) {
         if (service_state == service_state_t::STARTING) {
-            if (! waiting_for_deps) {
+            // If waiting for a dependency, or waiting for the console, we can interrupt start. Otherwise,
+            // we need to delegate to can_interrupt_start() (which can be overridden).
+            if (! waiting_for_deps && ! waiting_for_console) {
                 if (! can_interrupt_start()) {
                     // Well this is awkward: we're going to have to continue starting. We can stop once we've
                     // reached the started state.
@@ -561,6 +570,10 @@ void service_record::do_stop() noexcept
                     return;
                 }
             }
+            else if (waiting_for_console) {
+                services->unqueue_console(this);
+                waiting_for_console = false;
+            }
 
             // We must have had desired_state == STARTED.
             notify_listeners(service_event_t::STARTCANCELLED);
@@ -648,11 +661,13 @@ void service_record::unpin() noexcept
 
 void service_record::queue_for_console() noexcept
 {
+    waiting_for_console = true;
     services->append_console_queue(this);
 }
 
 void service_record::release_console() noexcept
 {
+    have_console = false;
     services->pull_console_queue();
 }