From 9276337bf68b8bf9422ec8000f2ef5c7838bc0b1 Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Fri, 19 Jan 2018 10:12:24 +0000 Subject: [PATCH] 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. --- src/baseproc-service.cc | 8 +------- src/includes/service.h | 8 +++++--- src/service.cc | 33 ++++++++++++++++++++++++--------- 3 files changed, 30 insertions(+), 19 deletions(-) diff --git a/src/baseproc-service.cc b/src/baseproc-service.cc index 3d0d01f..87f6088 100644 --- a/src/baseproc-service.cc +++ b/src/baseproc-service.cc @@ -149,7 +149,6 @@ bool base_process_service::start_ps_process(const std::vector &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 a11262f..4e9aafa 100644 --- 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 f5e3575..e5351ed 100644 --- 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(); } -- 2.25.1