From ccdf8b687d3c31da98f7b9778ecd83751b8a3e96 Mon Sep 17 00:00:00 2001 From: Davin McCall Date: Mon, 13 Apr 2020 12:12:13 +1000 Subject: [PATCH] Fix bug with activation count for auto-restart services This could result in shutdown failure. Also, when stopping such a process via dinitctl, the process would always restart if it had any dependents (including soft); fix that. --- src/proc-service.cc | 2 -- src/service.cc | 6 +++++- src/tests/proctests.cc | 24 +++++++++++++++++++++++- 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/src/proc-service.cc b/src/proc-service.cc index c761140..5d23c2f 100644 --- a/src/proc-service.cc +++ b/src/proc-service.cc @@ -281,7 +281,6 @@ void bgproc_service::handle_exit_status(bp_sys::exit_status exit_status) noexcep // This may be a "smooth recovery" where we are restarting the process while leaving the // service in the STARTED state. if (restarting && service_state == service_state_t::STARTED) { - //restarting = false; bool need_stop = false; if ((did_exit && exit_status.get_exit_status() != 0) || was_signalled) { need_stop = true; @@ -313,7 +312,6 @@ void bgproc_service::handle_exit_status(bp_sys::exit_status exit_status) noexcep return; } - //restarting = false; if (service_state == service_state_t::STARTING) { // POSIX requires that if the process exited clearly with a status code of 0, // the exit status value will be 0: diff --git a/src/service.cc b/src/service.cc index 18987c6..3ea862b 100644 --- a/src/service.cc +++ b/src/service.cc @@ -50,8 +50,10 @@ void service_record::stopped() noexcept force_stop = false; + // If we are to re-start, restarting should have been set true and desired_state should be STARTED. + // (A restart could be cancelled via a separately issued stop, including via a shutdown). restarting |= auto_restart; - bool will_restart = restarting && required_by > 0; + bool will_restart = restarting && desired_state == service_state_t::STARTED; if (restarting && ! will_restart) { notify_listeners(service_event_t::STARTCANCELLED); } @@ -493,6 +495,8 @@ void service_record::stop(bool bring_down) noexcept bring_down = true; } + // Set desired state to STOPPED, this will be set back to STARTED if there any hard dependents + // that want to restart. desired_state = service_state_t::STOPPED; if (bring_down && service_state != service_state_t::STOPPED diff --git a/src/tests/proctests.cc b/src/tests/proctests.cc index 395ec3e..b75eea3 100644 --- a/src/tests/proctests.cc +++ b/src/tests/proctests.cc @@ -251,6 +251,7 @@ void test_proc_term_restart2() assert(p.get_state() == service_state_t::STARTED); assert(event_loop.active_timers.size() == 0); + // simulate process terminating, should then be restarted: base_process_service_test::handle_exit(&p, 0); sset.process_queues(); @@ -268,11 +269,32 @@ void test_proc_term_restart2() assert(p.get_state() == service_state_t::STARTED); assert(event_loop.active_timers.size() == 0); + assert(sset.count_active_services() == 2); + + // Request stop, this time it should not restart: + p.stop(true); + sset.process_queues(); + base_process_service_test::handle_exit(&p, 0); + sset.process_queues(); + + assert(p.get_state() == service_state_t::STOPPED); + assert(event_loop.active_timers.size() == 0); + assert(sset.count_active_services() == 1); + + // simulate terminate dinit + sset.stop_all_services(); + + //base_process_service_test::handle_exit(&p, 0); + sset.process_queues(); + + assert(p.get_state() == service_state_t::STOPPED); + assert(event_loop.active_timers.size() == 0); + assert(sset.count_active_services() == 0); + sset.remove_service(&p); sset.remove_service(&b); } - // Termination via stop request void test_term_via_stop() { -- 2.25.1