From cdf109f427fdd3e584fa2203ad7d723756b2bb14 Mon Sep 17 00:00:00 2001 From: Cameron Craig Date: Fri, 30 Oct 2026 10:47:38 +0000 Subject: [PATCH 01/11] test: Add integration test --- tests/integration/BUILD | 1 + .../lm_shutdown_with_control_requests/BUILD | 54 +++++++++++ .../common.hpp | 26 +++++ .../config.json | 92 ++++++++++++++++++ .../control_client_test_driver.cpp | 94 +++++++++++++++++++ .../lm_control_requester.cpp | 71 ++++++++++++++ .../lm_shutdown_with_control_requests/test.py | 57 +++++++++++ 7 files changed, 395 insertions(+) create mode 100644 tests/integration/lm_shutdown_with_control_requests/BUILD create mode 100644 tests/integration/lm_shutdown_with_control_requests/common.hpp create mode 100644 tests/integration/lm_shutdown_with_control_requests/config.json create mode 100644 tests/integration/lm_shutdown_with_control_requests/control_client_test_driver.cpp create mode 100644 tests/integration/lm_shutdown_with_control_requests/lm_control_requester.cpp create mode 100644 tests/integration/lm_shutdown_with_control_requests/test.py diff --git a/tests/integration/BUILD b/tests/integration/BUILD index 596e56b99a..e5f8449f6e 100644 --- a/tests/integration/BUILD +++ b/tests/integration/BUILD @@ -22,6 +22,7 @@ test_suite( "//tests/integration/incorrect_config_non_reporting", "//tests/integration/lm_shutdown_during_rt_switch", "//tests/integration/lm_shutdown_during_switch_to_off", + "//tests/integration/lm_shutdown_with_control_requests", "//tests/integration/parallel_launch", "//tests/integration/process_complex_rep_failure", "//tests/integration/process_crash_monitoring", diff --git a/tests/integration/lm_shutdown_with_control_requests/BUILD b/tests/integration/lm_shutdown_with_control_requests/BUILD new file mode 100644 index 0000000000..51cfb499dd --- /dev/null +++ b/tests/integration/lm_shutdown_with_control_requests/BUILD @@ -0,0 +1,54 @@ +# ******************************************************************************* +# Copyright (c) 2026 Contributors to the Eclipse Foundation +# +# See the NOTICE file(s) distributed with this work for additional +# information regarding copyright ownership. +# +# This program and the accompanying materials are made available under the +# terms of the Apache License Version 2.0 which is available at +# https://www.apache.org/licenses/LICENSE-2.0 +# +# SPDX-License-Identifier: Apache-2.0 +# ******************************************************************************* +load("@rules_cc//cc:cc_binary.bzl", "cc_binary") +load("@rules_cc//cc:cc_library.bzl", "cc_library") +load("//tests/utils/bazel:integration.bzl", "integration_test") + +cc_library( + name = "common", + hdrs = ["common.hpp"], +) + +cc_binary( + name = "control_client_test_driver", + srcs = ["control_client_test_driver.cpp"], + deps = [ + ":common", + "//score/launch_manager:control_cc", + "//score/launch_manager:lifecycle_cc", + "//tests/utils/test_helper", + "@googletest//:gtest_main", + ], +) + +cc_binary( + name = "lm_control_requester", + srcs = ["lm_control_requester.cpp"], + deps = [ + ":common", + "//score/launch_manager:control_cc", + ], +) + +integration_test( + name = "lm_shutdown_with_control_requests", + timeout = "short", + srcs = ["test.py"], + binaries = [ + "//tests/utils/test_helper:process_hanging_on_sigterm", + ":control_client_test_driver", + ":lm_control_requester", + "//score/launch_manager", + ], + config = ":config.json", +) diff --git a/tests/integration/lm_shutdown_with_control_requests/common.hpp b/tests/integration/lm_shutdown_with_control_requests/common.hpp new file mode 100644 index 0000000000..162ae4dc88 --- /dev/null +++ b/tests/integration/lm_shutdown_with_control_requests/common.hpp @@ -0,0 +1,26 @@ +/******************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Apache License Version 2.0 which is available at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * SPDX-License-Identifier: Apache-2.0 + ********************************************************************************/ + +#ifndef SCORE_TESTS_INTEGRATION_LM_SHUTDOWN_WITH_CONTROL_REQUESTS_COMMON_HPP +#define SCORE_TESTS_INTEGRATION_LM_SHUTDOWN_WITH_CONTROL_REQUESTS_COMMON_HPP + +#include + +/// @brief Written by component_a when it has reported running (run_target_a is +/// active). +constexpr std::string_view a_started = "component_a_started"; + +/// @brief Written by component_a when it starts being terminated +constexpr std::string_view a_terminating = "component_a_terminating"; + +#endif // SCORE_TESTS_INTEGRATION_LM_SHUTDOWN_WITH_CONTROL_REQUESTS_COMMON_HPP diff --git a/tests/integration/lm_shutdown_with_control_requests/config.json b/tests/integration/lm_shutdown_with_control_requests/config.json new file mode 100644 index 0000000000..8a66468d0d --- /dev/null +++ b/tests/integration/lm_shutdown_with_control_requests/config.json @@ -0,0 +1,92 @@ +{ + "schema_version": 2, + "defaults": { + "deployment_config": { + "bin_dir": "/tmp/tests/lm_shutdown_with_control_requests", + "ready_timeout_ms": 1000, + "shutdown_timeout_ms": 1000, + "ready_recovery_action": { + "restart": { + "number_of_attempts": 0 + } + }, + "recovery_action": { + "switch_run_target": { + "run_target": "fallback_run_target" + } + }, + "environmental_variables": { + "LD_LIBRARY_PATH": "/opt/lib" + }, + "sandbox": { + "uid": 0, + "gid": 0, + "scheduling_policy": "SCHED_OTHER", + "scheduling_priority": 0 + } + }, + "component_properties": { + "application_profile": { + "application_type": "Reporting", + "is_self_terminating": false + }, + "ready_condition": { + "process_state": "Running" + } + } + }, + "components": { + "component_initial": { + "component_properties": { + "binary_name": "control_client_test_driver", + "application_profile": { + "application_type": "State_Manager", + "alive_supervision": { + "min_indications": 0 + } + } + }, + "deployment_config": { + "ready_timeout_ms": 1000, + "shutdown_timeout_ms": 5000, + "environmental_variables": { + "PROCESSIDENTIFIER": "control_client_test_driver" + } + } + }, + "component_a": { + "component_properties": { + "binary_name": "process_hanging_on_sigterm" + }, + "deployment_config": { + "shutdown_timeout_ms": 5000, + "environmental_variables": { + "PROCESSIDENTIFIER": "component_a" + } + } + } + }, + "run_targets": { + "Startup": { + "depends_on": [ + "component_initial" + ] + }, + "run_target_a": { + "depends_on": [ + "component_initial", + "component_a" + ] + }, + "Off": { + "depends_on": [] + } + }, + "initial_run_target": "Startup", + "alive_supervision": { + "evaluation_cycle_ms": 50 + }, + "fallback_run_target": { + "depends_on": [] + } +} diff --git a/tests/integration/lm_shutdown_with_control_requests/control_client_test_driver.cpp b/tests/integration/lm_shutdown_with_control_requests/control_client_test_driver.cpp new file mode 100644 index 0000000000..002d738e1c --- /dev/null +++ b/tests/integration/lm_shutdown_with_control_requests/control_client_test_driver.cpp @@ -0,0 +1,94 @@ +/******************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Apache License Version 2.0 which is available at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * SPDX-License-Identifier: Apache-2.0 + ********************************************************************************/ +#include +#include +#include + +#include "common.hpp" +#include "tests/utils/test_helper/test_helper.hpp" +#include "tests/utils/test_helper/test_runner.hpp" +#include +#include + +using namespace score::mw::lifecycle; + +TEST(LmShutdownControlRequests, ControlClient) +{ + ASSERT_TRUE(check_clean({a_started, a_terminating})); + std::unique_ptr client; + + TEST_STEP("Create LmControl client") + { + auto client_result = ILmControl::Create("StateManager/LaunchManager/Instance"); + ASSERT_TRUE(client_result.has_value()) << client_result.error().Message(); + client = std::move(client_result).value(); + } + + TEST_STEP("Register run target activation callback") + { + const auto result = client->register_run_target_activation_callback(push_event); + ASSERT_TRUE(result.has_value()); + } + + TEST_STEP("Report running") + { + report_running(); + } + + pop_event([](RunTargetActivationSource source, RunTargetName target) { + TEST_STEP("Callback for Run Target Startup") + { + EXPECT_EQ(source, RunTargetActivationSource::kInitialActivation); + EXPECT_EQ(target, "Startup"); + } + }); + + TEST_STEP("Activate run_target_a") + { + const auto result = client->activate_run_target("run_target_a", true); + EXPECT_TRUE(result.has_value()) << result.error().Message(); + } + + TEST_STEP("Wait for run_target_a activation") + { + pop_event([](RunTargetActivationSource source, RunTargetName target) { + TEST_STEP("Callback for Run Target run_target_a") + { + EXPECT_EQ(source, RunTargetActivationSource::kStateManagerRequest); + EXPECT_EQ(target, "run_target_a"); + } + }); + } + + TEST_STEP("Verify activation of run_target_a") + { + EXPECT_TRUE(std::filesystem::exists(a_started)) << "component_a was not started"; + } + + TEST_STEP("Request switch to Off") + { + const auto result = client->activate_run_target("Off", true); + EXPECT_TRUE(result.has_value()) << result.error().Message(); + } + + // Block until the launch manager terminates us as part of its own shutdown + TestRunner::WaitForTermination(); +} + +int main() +{ + TestRunner::Init(); + const auto result = TestRunner::RunTests(); + TestRunner::EndTest(); + return result; +} diff --git a/tests/integration/lm_shutdown_with_control_requests/lm_control_requester.cpp b/tests/integration/lm_shutdown_with_control_requests/lm_control_requester.cpp new file mode 100644 index 0000000000..a91279bdf6 --- /dev/null +++ b/tests/integration/lm_shutdown_with_control_requests/lm_control_requester.cpp @@ -0,0 +1,71 @@ +/******************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Apache License Version 2.0 which is available at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * SPDX-License-Identifier: Apache-2.0 + ********************************************************************************/ +#include +#include +#include +#include + +#include "common.hpp" +#include + +using namespace score::mw::lifecycle; + +namespace +{ +constexpr auto kTimeout = std::chrono::seconds(20); +constexpr std::uint64_t kMaxFailures = 500; +} // namespace + +// Keeps sending LmControl requests (get_active_run_target and an "Off" activation) to the +// Launch Manager once run_target_a is active, until the Launch Manager stops answering. +// It is not owned by the Launch Manager, so it keeps sending while the Launch Manager shuts +// down, including after the control client has been terminated and while the +// ProcessGroupManager is being destroyed. +int main() +{ + while (!std::filesystem::exists(a_started)) + { + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + } + + const auto deadline = std::chrono::steady_clock::now() + kTimeout; + std::unique_ptr client; + std::uint64_t failure_count = 0; + std::uint64_t request_count = 0; + + while (std::chrono::steady_clock::now() < deadline && failure_count < kMaxFailures) + { + if (!client) + { + auto client_result = ILmControl::Create("StateManager/LaunchManager/Instance"); + if (!client_result.has_value()) + { + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + continue; + } + client = std::move(client_result).value(); + } + + const bool active_ok = client->get_active_run_target().has_value(); + const bool activation_ok = client->activate_run_target("Off", true).has_value(); + request_count += 2; + + if (!(active_ok && activation_ok)) + { + failure_count++; + } + } + + std::cout << "lm_control_requester sent " << request_count << " requests" << std::endl; + return 0; +} diff --git a/tests/integration/lm_shutdown_with_control_requests/test.py b/tests/integration/lm_shutdown_with_control_requests/test.py new file mode 100644 index 0000000000..20c4171e57 --- /dev/null +++ b/tests/integration/lm_shutdown_with_control_requests/test.py @@ -0,0 +1,57 @@ +# ******************************************************************************* +# Copyright (c) 2026 Contributors to the Eclipse Foundation +# +# See the NOTICE file(s) distributed with this work for additional +# information regarding copyright ownership. +# +# This program and the accompanying materials are made available under the +# terms of the Apache License Version 2.0 which is available at +# https://www.apache.org/licenses/LICENSE-2.0 +# +# SPDX-License-Identifier: Apache-2.0 +# ******************************************************************************* +from tests.utils.testing_utils.setup_test import setup_test +from tests.utils.testing_utils.test_results import assert_test_results +from attribute_plugin import add_test_properties + + +@add_test_properties( + fully_verifies=[], + partially_verifies=[ + "comp_req__launch_man__launcher_exit_shutdown", + ], + test_type="requirements-based", + derivation_technique="requirements-analysis", +) +def test_lm_shutdown_with_control_requests( + target, setup_test, assert_test_results, remote_test_dir +): + """ + Objective: Verifies that the Launch Manager handles LmControl requests that arrive + while it is shutting down without hanging or crashing. + + lm_control_requester keeps sending GetActiveRunTarget and ActivateRunTarget requests. + The control client switches to the "Off" Run Target. + We then delibrately hang in component_a, while terminating. + This forces the test to send a SIGTERM to the launch manager. + The lm_control_requester outlives the control client, so requests keep arriving + until the launch manager has destroyed the ProcessGroupManager. + + Expected Behaviour: The launch manager shuts down and exits cleanly. + """ + + lm = target.execute_async( + str(remote_test_dir / "launch_manager"), + args=["-c", str(remote_test_dir / "etc/config.bin")], + cwd=str(remote_test_dir), + ) + requester = target.execute_async( + str(remote_test_dir / "lm_control_requester"), + args=[], + cwd=str(remote_test_dir), + ) + + assert lm.wait(15) == 0 + assert requester.wait(30) == 0 + + assert_test_results({"control_client_test_driver.xml", "component_a.xml"}) From 874d50cee2255986652b3146639e3bd81e5b1d43 Mon Sep 17 00:00:00 2001 From: Cameron Craig Date: Fri, 30 Oct 2026 10:48:28 +0000 Subject: [PATCH 02/11] fix: Complete slots to prevent crash/hang --- .../src/process_group_manager/details/graph.cpp | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/graph.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/graph.cpp index 57d7a47fa7..8fba76784b 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/graph.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/graph.cpp @@ -385,6 +385,23 @@ void Graph::handleComponentEvent(const ComponentEvent& event) IComponent& failingComponent = componentOf(nodes_[data.process_identifier]); failingComponent.handleSupervisionFailure(data.failure_timestamp); } + // GetActiveRunTarget & SetRequestedRunTarget are only reached while shutting down, + // when the graph is no longer serving run target requests. + // The caller blocks on the slot, so it must always be completed. + else if constexpr (std::is_same_v) + { + if (data.completion != nullptr) + { + data.completion->complete(score::MakeUnexpected(ExecErrc::kFailed)); + } + } + else if constexpr (std::is_same_v) + { + if (data.completion != nullptr) + { + data.completion->complete(score::MakeUnexpected(ExecErrc::kFailed)); + } + } }, event); } From c4a245facf6d3f12cbedb1cac4582c95c7965f2c Mon Sep 17 00:00:00 2001 From: Cameron Craig Date: Fri, 30 Oct 2026 11:22:27 +0000 Subject: [PATCH 03/11] fix: Handle new error code in assertion --- .../src/daemon/src/control/control_provider.hpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/score/launch_manager/src/daemon/src/control/control_provider.hpp b/score/launch_manager/src/daemon/src/control/control_provider.hpp index a048d56ada..f8cc3c73a9 100644 --- a/score/launch_manager/src/daemon/src/control/control_provider.hpp +++ b/score/launch_manager/src/daemon/src/control/control_provider.hpp @@ -182,9 +182,11 @@ class BasicControlProvider const score::Result result = graph_->getActiveRunTarget(); if (!result.has_value()) { + // kFailed can be returned on shutdown SCORE_LANGUAGE_FUTURECPP_ASSERT_MESSAGE( - static_cast(*result.error()) == ExecErrc::kActivationInProgress, - "Impossible to communicate errors other than ExecErrc::kActivationInProgress to the client"); + static_cast(*result.error()) == ExecErrc::kActivationInProgress || + static_cast(*result.error()) == ExecErrc::kFailed, + "Impossible to communicate errors other than kActivationInProgress & kFailed to the client"); response = GetActiveRunTargetResponse{QueryStatus::kNotAvailable, RunTargetName("")}; return; From c277e8b4eb40355b2fdba580ac234a0ca0c23b5d Mon Sep 17 00:00:00 2001 From: Cameron Craig Date: Fri, 30 Oct 2026 11:23:09 +0000 Subject: [PATCH 04/11] test: Add unit tests for graph --- .../details/graph_UT.cpp | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/graph_UT.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/graph_UT.cpp index fbd4c85985..ea9d1f1a37 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/graph_UT.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/graph_UT.cpp @@ -268,6 +268,52 @@ TEST_F(GraphOrdinaryTransitionTest, simpleDeactivationTransition) EXPECT_EQ(graph_->getRequestedRunTarget(), target); } +TEST_F(GraphOrdinaryTransitionTest, GivenGetActiveRunTargetEvent_WhenUnblockComponentEventCalled_ThenSlotIsCompleted) +{ + RecordProperty( + "Description", + "Given a GetActiveRunTarget event. " + "When the event is passed to unblockComponentEvent(). " + "Then the slot is completed with an error. " + "And the wait() does not block."); + + // Given + constexpr auto COMPLETION_REASON = ExecErrc::kFailed; + CompletionSlot> slot; + auto event = GetActiveRunTarget{&slot}; + + // When + graph_->unblockComponentEvent(event, COMPLETION_REASON); + + // Then + auto completion = slot.wait(); + ASSERT_FALSE(completion.has_value()); + EXPECT_EQ(completion.error(), COMPLETION_REASON); +} + +TEST_F(GraphOrdinaryTransitionTest, GivenSetRequestedRunTarget_WhenUnblockComponentEventCalled_ThenSlotIsCompleted) +{ + RecordProperty( + "Description", + "Given a SetRequestedRunTarget event. " + "When the event is passed to unblockComponentEvent(). " + "Then the slot is completed with an error. " + "And the wait() does not block."); + + // Given + constexpr auto COMPLETION_REASON = ExecErrc::kFailed; + CompletionSlot> slot; + auto event = SetRequestedRunTarget{IdentifierHash{run_target_name(0)}, &slot}; + + // When + graph_->unblockComponentEvent(event, COMPLETION_REASON); + + // Then + auto completion = slot.wait(); + ASSERT_FALSE(completion.has_value()); + EXPECT_EQ(completion.error(), COMPLETION_REASON); +} + class GraphActivationSourceTest : public GraphTest { protected: From 2bea0fb7dbdf6706d0767b01e425014a2174708e Mon Sep 17 00:00:00 2001 From: Cameron Craig Date: Fri, 30 Oct 2026 13:14:02 +0000 Subject: [PATCH 05/11] fix: Measure the time when calculating timeout --- .../process_group_manager/process_group_manager.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/score/launch_manager/src/daemon/src/process_group_manager/process_group_manager.cpp b/score/launch_manager/src/daemon/src/process_group_manager/process_group_manager.cpp index f572d6fa99..aec100f938 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/process_group_manager.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/process_group_manager.cpp @@ -269,16 +269,19 @@ void ProcessGroupManager::allProcessGroupsOff() // SupervisionFailure is intentionally ignored here so recovery transitions do not // fight the forced transition to Off. auto waitForStateCompletion = [this](GraphState state_to_be_completed, int32_t max_wait_ms) -> bool { - constexpr int32_t kSleepIntervalMs = 10; + constexpr int32_t kMaxSleepIntervalMs = 10; auto has_state = [this, state_to_be_completed]() { return graph_->getState() == state_to_be_completed; }; - int32_t remaining_ms = max_wait_ms; - while (has_state() && (remaining_ms > 0)) + // A flood of events makes event_queue_->waitForEvents() return immediately, + // as the queue will never be empty. + // We cannot rely on a kMaxSleepIntervalMs delay per iteration + const auto deadline = std::chrono::steady_clock::now() + std::chrono::milliseconds{max_wait_ms}; + while (has_state() && (std::chrono::steady_clock::now() < deadline)) { - static_cast(event_queue_->waitForEvents(std::chrono::milliseconds(kSleepIntervalMs))); + static_cast(event_queue_->waitForEvents(std::chrono::milliseconds(kMaxSleepIntervalMs))); while (auto event = event_queue_->getNextEvent()) { if (std::holds_alternative(*event)) @@ -287,8 +290,6 @@ void ProcessGroupManager::allProcessGroupsOff() } graph_->handleComponentEvent(*event); } - - remaining_ms -= kSleepIntervalMs; } return !has_state(); From cdd40b11ff4e7d9b0171298955d528416e6b0af2 Mon Sep 17 00:00:00 2001 From: Cameron Craig Date: Fri, 30 Oct 2026 16:50:54 +0000 Subject: [PATCH 06/11] refactor: Add a new error code --- .../daemon/src/control/control_provider.hpp | 5 ++-- .../src/control/control_provider_UT.cpp | 6 ++--- .../process_group_manager/details/graph.cpp | 23 +++++++++++++++++++ .../process_group_manager/details/graph.hpp | 6 +++++ .../process_group_manager.cpp | 11 ++++++++- score/launch_manager/src/execution_error.h | 1 + 6 files changed, 45 insertions(+), 7 deletions(-) diff --git a/score/launch_manager/src/daemon/src/control/control_provider.hpp b/score/launch_manager/src/daemon/src/control/control_provider.hpp index f8cc3c73a9..6459292bed 100644 --- a/score/launch_manager/src/daemon/src/control/control_provider.hpp +++ b/score/launch_manager/src/daemon/src/control/control_provider.hpp @@ -182,11 +182,10 @@ class BasicControlProvider const score::Result result = graph_->getActiveRunTarget(); if (!result.has_value()) { - // kFailed can be returned on shutdown SCORE_LANGUAGE_FUTURECPP_ASSERT_MESSAGE( static_cast(*result.error()) == ExecErrc::kActivationInProgress || - static_cast(*result.error()) == ExecErrc::kFailed, - "Impossible to communicate errors other than kActivationInProgress & kFailed to the client"); + static_cast(*result.error()) == ExecErrc::kShuttingDown, + "Impossible to communicate errors other than kActivationInProgress & kShuttingDown to the client"); response = GetActiveRunTargetResponse{QueryStatus::kNotAvailable, RunTargetName("")}; return; diff --git a/score/launch_manager/src/daemon/src/control/control_provider_UT.cpp b/score/launch_manager/src/daemon/src/control/control_provider_UT.cpp index 4ce587b162..cb098a7641 100644 --- a/score/launch_manager/src/daemon/src/control/control_provider_UT.cpp +++ b/score/launch_manager/src/daemon/src/control/control_provider_UT.cpp @@ -557,21 +557,21 @@ TEST_F(ControlProviderUT, GetActiveRunTarget_UnexpectedError_Aborts) { RecordProperty( "Description", - "If getActiveRunTarget() returns any error other than kActivationInProgress " + "If getActiveRunTarget() returns any error other than kActivationInProgress or kShuttingDown " "the handler hits an internal assertion and the process terminates, because " "no other error code can be communicated to the client."); auto provider = MakeProvider(); ASSERT_TRUE(on_get_active_) << "get_active_run_target handler was not registered"; - EXPECT_CALL(graph_, getActiveRunTarget()).WillRepeatedly(Return(MakeUnexpected(ExecErrc::kFailed))); + EXPECT_CALL(graph_, getActiveRunTarget()).WillRepeatedly(Return(MakeUnexpected(ExecErrc::kMetaModelError))); EXPECT_DEATH( { GetActiveRunTargetResponse response{}; on_get_active_(response); }, - "Impossible to communicate errors other than ExecErrc::kActivationInProgress to the client"); + "Impossible to communicate errors other than kActivationInProgress & kShuttingDown to the client"); } // ============================================================================ diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/graph.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/graph.cpp index 8fba76784b..1708c180ad 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/graph.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/graph.cpp @@ -406,6 +406,29 @@ void Graph::handleComponentEvent(const ComponentEvent& event) event); } +void Graph::unblockComponentEvent(const ComponentEvent& event, const ExecErrc reason) +{ + std::visit( + [this, reason](const auto& data) { + using T = std::decay_t; + if constexpr (std::is_same_v) + { + if (data.completion != nullptr) + { + data.completion->complete(score::MakeUnexpected(reason)); + } + } + else if constexpr (std::is_same_v) + { + if (data.completion != nullptr) + { + data.completion->complete(score::MakeUnexpected(reason)); + } + } + }, + event); +} + void Graph::nodeExecuted(IdentifierHash node, IComponent::RequestResult result) { bool was_last_in_queue = --jobs_in_progress_ == 0; diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/graph.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/graph.hpp index 9769aba735..c6abe99eb4 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/graph.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/graph.hpp @@ -184,6 +184,12 @@ class Graph final /// @param event The event to process. void handleComponentEvent(const ComponentEvent& event); + /// @brief Completes the slot attached to certain events. + /// Does not follow through on the request, the event is ignored. + /// @param event The event to complete. + /// @param event The error code containing the reason for ignoring the request. + void unblockComponentEvent(const ComponentEvent& event, const ExecErrc reason); + /// @brief Cancel the current transition because a new state has been requested. /// Sets the graph state to kAborting. /// If no jobs are in progress, transitions immediately to kUndefinedState. diff --git a/score/launch_manager/src/daemon/src/process_group_manager/process_group_manager.cpp b/score/launch_manager/src/daemon/src/process_group_manager/process_group_manager.cpp index aec100f938..950fe62d41 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/process_group_manager.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/process_group_manager.cpp @@ -288,7 +288,16 @@ void ProcessGroupManager::allProcessGroupsOff() { continue; } - graph_->handleComponentEvent(*event); + else if ( + std::holds_alternative(*event) || + std::holds_alternative(*event)) + { + graph_->unblockComponentEvent(*event, ExecErrc::kShuttingDown); + } + else + { + graph_->handleComponentEvent(*event); + } } } diff --git a/score/launch_manager/src/execution_error.h b/score/launch_manager/src/execution_error.h index 887e2c6a47..04589ca82f 100644 --- a/score/launch_manager/src/execution_error.h +++ b/score/launch_manager/src/execution_error.h @@ -40,6 +40,7 @@ enum class ExecErrc : score::result::ErrorCode kRequestQueueIsFull = 13, ///< The activation request queue is full; the request was discarded kRunTargetDoesntExist = 14, ///< The requested Run Target does not exist in the current configuration kNotImplemented = 15, ///< The requested functionality is not yet implemented + kShuttingDown = 16, ///< The request could not be completed because launch manager is shutting down }; class ExecErrorDomain final : public score::result::ErrorDomain From 881ad1a9655f4eeffa1e6b8f558f15d446f622d8 Mon Sep 17 00:00:00 2001 From: Cameron Craig Date: Mon, 2 Nov 2026 11:34:55 +0000 Subject: [PATCH 07/11] chore: Wait for client connection in test, log the dtor --- .../control_client_test_driver.cpp | 5 ++ .../lm_control_requester.cpp | 60 +++++++++++-------- 2 files changed, 41 insertions(+), 24 deletions(-) diff --git a/tests/integration/lm_shutdown_with_control_requests/control_client_test_driver.cpp b/tests/integration/lm_shutdown_with_control_requests/control_client_test_driver.cpp index 002d738e1c..164d9beda6 100644 --- a/tests/integration/lm_shutdown_with_control_requests/control_client_test_driver.cpp +++ b/tests/integration/lm_shutdown_with_control_requests/control_client_test_driver.cpp @@ -40,6 +40,11 @@ TEST(LmShutdownControlRequests, ControlClient) ASSERT_TRUE(result.has_value()); } + TEST_STEP("Wait for client connection") + { + ASSERT_TRUE(client->wait_for_connection().has_value()); + } + TEST_STEP("Report running") { report_running(); diff --git a/tests/integration/lm_shutdown_with_control_requests/lm_control_requester.cpp b/tests/integration/lm_shutdown_with_control_requests/lm_control_requester.cpp index a91279bdf6..46d8442b0f 100644 --- a/tests/integration/lm_shutdown_with_control_requests/lm_control_requester.cpp +++ b/tests/integration/lm_shutdown_with_control_requests/lm_control_requester.cpp @@ -26,11 +26,11 @@ constexpr auto kTimeout = std::chrono::seconds(20); constexpr std::uint64_t kMaxFailures = 500; } // namespace -// Keeps sending LmControl requests (get_active_run_target and an "Off" activation) to the -// Launch Manager once run_target_a is active, until the Launch Manager stops answering. -// It is not owned by the Launch Manager, so it keeps sending while the Launch Manager shuts -// down, including after the control client has been terminated and while the -// ProcessGroupManager is being destroyed. +/// @brief Keeps sending LmControl requests (get_active_run_target and an "Off" activation) to the +/// Launch Manager once run_target_a is active, until the Launch Manager stops answering. +/// It is not owned by the Launch Manager, so it keeps sending while the Launch Manager shuts +/// down, including after the control client has been terminated and while the +/// ProcessGroupManager is being destroyed. int main() { while (!std::filesystem::exists(a_started)) @@ -39,33 +39,45 @@ int main() } const auto deadline = std::chrono::steady_clock::now() + kTimeout; - std::unique_ptr client; - std::uint64_t failure_count = 0; - std::uint64_t request_count = 0; + std::uint64_t connect_attempt_count = 0U; + std::uint64_t failure_count = 0U; + std::uint64_t request_count = 0U; - while (std::chrono::steady_clock::now() < deadline && failure_count < kMaxFailures) - { - if (!client) + { // client scope + std::unique_ptr client; + + while (std::chrono::steady_clock::now() < deadline && failure_count < kMaxFailures) { - auto client_result = ILmControl::Create("StateManager/LaunchManager/Instance"); - if (!client_result.has_value()) + if (!client) { - std::this_thread::sleep_for(std::chrono::milliseconds(1)); - continue; + std::cout << "Connecting client\n"; + auto client_result = ILmControl::Create("StateManager/LaunchManager/Instance"); + ++connect_attempt_count; + + if (!client_result.has_value()) + { + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + continue; + } + + std::cout << "Client connected after " << connect_attempt_count << " attempts\n"; + client = std::move(client_result).value(); } - client = std::move(client_result).value(); - } - const bool active_ok = client->get_active_run_target().has_value(); - const bool activation_ok = client->activate_run_target("Off", true).has_value(); - request_count += 2; + const bool active_ok = client->get_active_run_target().has_value(); + const bool activation_ok = client->activate_run_target("Off", true).has_value(); + request_count += 2; - if (!(active_ok && activation_ok)) - { - failure_count++; + if (!(active_ok && activation_ok)) + { + failure_count++; + } } + + std::cout << "sent " << request_count << " requests\n"; + std::cout << "destroying client\n"; } - std::cout << "lm_control_requester sent " << request_count << " requests" << std::endl; + std::cout << "client destroyed, exiting\n"; return 0; } From 423d9026cc50b1de05b66857516ca08d42fda956 Mon Sep 17 00:00:00 2001 From: Cameron Craig Date: Wed, 4 Nov 2026 11:31:11 +0000 Subject: [PATCH 08/11] refactor: Remove unused branches --- .../src/process_group_manager/details/graph.cpp | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/graph.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/graph.cpp index 1708c180ad..af8ce42e90 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/graph.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/graph.cpp @@ -385,23 +385,6 @@ void Graph::handleComponentEvent(const ComponentEvent& event) IComponent& failingComponent = componentOf(nodes_[data.process_identifier]); failingComponent.handleSupervisionFailure(data.failure_timestamp); } - // GetActiveRunTarget & SetRequestedRunTarget are only reached while shutting down, - // when the graph is no longer serving run target requests. - // The caller blocks on the slot, so it must always be completed. - else if constexpr (std::is_same_v) - { - if (data.completion != nullptr) - { - data.completion->complete(score::MakeUnexpected(ExecErrc::kFailed)); - } - } - else if constexpr (std::is_same_v) - { - if (data.completion != nullptr) - { - data.completion->complete(score::MakeUnexpected(ExecErrc::kFailed)); - } - } }, event); } From 7a4f488a37075decc8599a0e4f4d034bb4742eb7 Mon Sep 17 00:00:00 2001 From: Cameron Craig Date: Wed, 4 Nov 2026 11:35:19 +0000 Subject: [PATCH 09/11] docs: Update comments --- .../src/daemon/src/process_group_manager/details/graph.hpp | 2 +- .../src/process_group_manager/process_group_manager.cpp | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/graph.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/graph.hpp index c6abe99eb4..175bd90b39 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/graph.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/graph.hpp @@ -187,7 +187,7 @@ class Graph final /// @brief Completes the slot attached to certain events. /// Does not follow through on the request, the event is ignored. /// @param event The event to complete. - /// @param event The error code containing the reason for ignoring the request. + /// @param reason The error code containing the reason for ignoring the request. void unblockComponentEvent(const ComponentEvent& event, const ExecErrc reason); /// @brief Cancel the current transition because a new state has been requested. diff --git a/score/launch_manager/src/daemon/src/process_group_manager/process_group_manager.cpp b/score/launch_manager/src/daemon/src/process_group_manager/process_group_manager.cpp index 950fe62d41..c3119499f4 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/process_group_manager.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/process_group_manager.cpp @@ -275,9 +275,7 @@ void ProcessGroupManager::allProcessGroupsOff() return graph_->getState() == state_to_be_completed; }; - // A flood of events makes event_queue_->waitForEvents() return immediately, - // as the queue will never be empty. - // We cannot rely on a kMaxSleepIntervalMs delay per iteration + // We cannot rely on the kMaxSleepIntervalMs delay actually elapsing, so we check the clock each iteration const auto deadline = std::chrono::steady_clock::now() + std::chrono::milliseconds{max_wait_ms}; while (has_state() && (std::chrono::steady_clock::now() < deadline)) { @@ -288,6 +286,7 @@ void ProcessGroupManager::allProcessGroupsOff() { continue; } + // We need to complete any slots here to prevent them causing a hang else if ( std::holds_alternative(*event) || std::holds_alternative(*event)) From 335177edf1cc494ddc39e1d931e658ed41ec2144 Mon Sep 17 00:00:00 2001 From: Cameron Craig Date: Wed, 4 Nov 2026 14:36:46 +0000 Subject: [PATCH 10/11] test: Add missing coverage --- .../details/graph_UT.cpp | 99 +++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/graph_UT.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/graph_UT.cpp index ea9d1f1a37..538524e8cf 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/graph_UT.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/graph_UT.cpp @@ -268,6 +268,46 @@ TEST_F(GraphOrdinaryTransitionTest, simpleDeactivationTransition) EXPECT_EQ(graph_->getRequestedRunTarget(), target); } +TEST_F(GraphOrdinaryTransitionTest, GivenGetActiveRunTargetEvent_WhenHandleComponentEventCalled_ThenAssert) +{ + RecordProperty( + "Description", + "Given a GetActiveRunTarget event. " + "When the event is passed to handleComponentEvent(). " + "Then the program asserts in debug builds, and does not assert in release builds."); + + // Given + CompletionSlot> slot; + auto event = GetActiveRunTarget{&slot}; + +#ifndef NDEBUG + // When + EXPECT_DEATH({ graph_->handleComponentEvent(event); }, "Assertion failed"); +#else + graph_->handleComponentEvent(event); +#endif // NDEBUG +} + +TEST_F(GraphOrdinaryTransitionTest, GivenSetRequestedRunTargetEvent_WhenHandleComponentEventCalled_ThenAssert) +{ + RecordProperty( + "Description", + "Given a SetRequestedRunTarget event. " + "When the event is passed to handleComponentEvent(). " + "Then the program asserts in debug builds, and does not assert in release builds."); + + // Given + CompletionSlot> slot; + auto event = SetRequestedRunTarget{IdentifierHash{run_target_name(0)}, &slot}; + +#ifndef NDEBUG + // When + EXPECT_DEATH({ graph_->handleComponentEvent(event); }, "Assertion failed"); +#else + graph_->handleComponentEvent(event); +#endif // NDEBUG +} + TEST_F(GraphOrdinaryTransitionTest, GivenGetActiveRunTargetEvent_WhenUnblockComponentEventCalled_ThenSlotIsCompleted) { RecordProperty( @@ -314,6 +354,65 @@ TEST_F(GraphOrdinaryTransitionTest, GivenSetRequestedRunTarget_WhenUnblockCompon EXPECT_EQ(completion.error(), COMPLETION_REASON); } +TEST_F( + GraphOrdinaryTransitionTest, + GivenGetActiveRunTargetWithNullSlotEvent_WhenUnblockComponentEventCalled_ThenNullCheck) +{ + RecordProperty( + "Description", + "Given a GetActiveRunTarget event with a nullptr completion slot. " + "When the event is passed to unblockComponentEvent(). " + "Then the nullptr slot is detected. " + "And no nullptr dereference occurs."); + + // Given + constexpr auto COMPLETION_REASON = ExecErrc::kFailed; + auto event = GetActiveRunTarget{nullptr}; + + // When, Then + graph_->unblockComponentEvent(event, COMPLETION_REASON); +} + +TEST_F( + GraphOrdinaryTransitionTest, + GivenSetRequestedRunTargetWithNullSlot_WhenUnblockComponentEventCalled_ThenNullCheck) +{ + RecordProperty( + "Description", + "Given a SetRequestedRunTarget event with a nullptr completion slot. " + "When the event is passed to unblockComponentEvent(). " + "Then the nullptr slot is detected. " + "And no nullptr dereference occurs."); + + // Given + constexpr auto COMPLETION_REASON = ExecErrc::kFailed; + auto event = SetRequestedRunTarget{IdentifierHash{run_target_name(0)}, nullptr}; + + // When, Then + graph_->unblockComponentEvent(event, COMPLETION_REASON); +} + +TEST_F(GraphOrdinaryTransitionTest, GivenInvalidEvent_WhenUnblockComponentEventCalled_ThenAssert) +{ + RecordProperty( + "Description", + "Given a ActivationSuccessful event (which is not handled by unblockComponentEvent). " + "When the event is passed to unblockComponentEvent(). " + "Then the program asserts in debug builds, and does not assert in release builds."); + + // Given + constexpr auto COMPLETION_REASON = ExecErrc::kFailed; + auto event = ActivationSuccessful{IdentifierHash{process_name(0)}}; + +#ifndef NDEBUG + // When, Then + EXPECT_DEATH({ graph_->unblockComponentEvent(event, COMPLETION_REASON); }, "Assertion failed"); +#else + // When, Then + graph_->unblockComponentEvent(event, COMPLETION_REASON); +#endif // NDEBUG +} + class GraphActivationSourceTest : public GraphTest { protected: From e5a605a8767c4d1a84bb68cc5abed9fc07ceed70 Mon Sep 17 00:00:00 2001 From: Cameron Craig Date: Wed, 4 Nov 2026 14:37:25 +0000 Subject: [PATCH 11/11] fix: Add asserts to prevent misuse --- .../process_group_manager/details/graph.cpp | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/graph.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/graph.cpp index af8ce42e90..f238d89adc 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/graph.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/graph.cpp @@ -34,6 +34,11 @@ namespace score::mw::lifecycle::internal namespace { +/// @brief False value dependent on T, to ensure evaluation is delayed until template evaluation, +/// preventing static_asserts from triggering early +template +constexpr bool always_false_v = false; + /// @brief Creates a dependency graph from the configuration. /// @param config The configuration containing components and Run Targets. /// @param process_handling The interfaces used to start, stop and report on the OS processes. @@ -385,6 +390,18 @@ void Graph::handleComponentEvent(const ComponentEvent& event) IComponent& failingComponent = componentOf(nodes_[data.process_identifier]); failingComponent.handleSupervisionFailure(data.failure_timestamp); } + else if constexpr (std::is_same_v || std::is_same_v) + { + // These events are handled by + // handleGetActiveRunTarget/handleSetRequestedRunTarget/unblockComponentEvent. + SCORE_LANGUAGE_FUTURECPP_UNREACHABLE_MESSAGE( + "It is a programmer error to call handleComponentEvent with " + "GetActiveRunTarget|SetRequestedRunTarget."); + } + else + { + static_assert(always_false_v, "Unhandled type in std::visit!"); + } }, event); } @@ -408,6 +425,12 @@ void Graph::unblockComponentEvent(const ComponentEvent& event, const ExecErrc re data.completion->complete(score::MakeUnexpected(reason)); } } + else + { + SCORE_LANGUAGE_FUTURECPP_UNREACHABLE_MESSAGE( + "It is a programmer error to call unblockComponentEvent with anything other than " + "GetActiveRunTarget|SetRequestedRunTarget."); + } }, event); }