From 75466a94b4bd6dcd0584159d4c017cf5182890fe Mon Sep 17 00:00:00 2001 From: Cameron Craig Date: Thu, 15 Oct 2026 08:30:43 +0100 Subject: [PATCH 01/10] fix: Set max number_of_attempts to 254 --- .../docs/product_documentation/configuration.rst | 3 +-- .../src/configuration/config_schema/launch_manager.schema.json | 1 + .../src/daemon/src/configuration/details/lm_flatcfg.fbs | 2 +- .../src/daemon/src/configuration/recovery_action_config.hpp | 2 +- .../src/process_group_manager/details/process_info_node.cpp | 3 ++- 5 files changed, 6 insertions(+), 5 deletions(-) diff --git a/score/launch_manager/docs/product_documentation/configuration.rst b/score/launch_manager/docs/product_documentation/configuration.rst index 52e04a77de..dbef803815 100644 --- a/score/launch_manager/docs/product_documentation/configuration.rst +++ b/score/launch_manager/docs/product_documentation/configuration.rst @@ -129,7 +129,7 @@ restart (object, optional) restart.number_of_attempts (integer, optional) Description: Specifies the maximum number of restart attempts before the **Launch Manager** concludes that recovery cannot succeed for the component. - Constraint: Must be 0 or greater. + Constraint: Must be between 0 and 254 inclusive. restart.delay_before_restart_ms (integer, optional) Description: Specifies the delay duration, in milliseconds, that the **Launch Manager** waits before initiating a restart attempt. @@ -555,4 +555,3 @@ Given that the **Launch Manager** supports multiple levels of default values, sp 1. If a configuration value is not explicitly specified at a specific location (e.g., within an individual component's definition, a **Run Target's** definition, or a root-level property like ``alive_supervision``), the **Launch Manager** will first attempt to use the corresponding value from the user-defined ``defaults`` section. 2. If the value is also not specified within the user-defined ``defaults`` section, then the **Launch Manager** will apply the S-CORE standard default value for that option. - diff --git a/score/launch_manager/src/daemon/src/configuration/config_schema/launch_manager.schema.json b/score/launch_manager/src/daemon/src/configuration/config_schema/launch_manager.schema.json index dad45d62f2..926ada9f70 100644 --- a/score/launch_manager/src/daemon/src/configuration/config_schema/launch_manager.schema.json +++ b/score/launch_manager/src/daemon/src/configuration/config_schema/launch_manager.schema.json @@ -150,6 +150,7 @@ "number_of_attempts": { "type": "integer", "minimum": 0, + "maximum": 254, "description": "Specifies the maximum number of restart attempts before the Launch Manager concludes that recovery cannot succeed." }, "delay_before_restart_ms": { diff --git a/score/launch_manager/src/daemon/src/configuration/details/lm_flatcfg.fbs b/score/launch_manager/src/daemon/src/configuration/details/lm_flatcfg.fbs index 8da36cb7e4..a6fe3e6d01 100644 --- a/score/launch_manager/src/daemon/src/configuration/details/lm_flatcfg.fbs +++ b/score/launch_manager/src/daemon/src/configuration/details/lm_flatcfg.fbs @@ -93,7 +93,7 @@ table ComponentProperties { // Recovery action that restarts the POSIX process associated with a component. table RestartAction { // Maximum number of restart attempts before recovery is considered failed. - number_of_attempts:uint32 = null; // required + number_of_attempts:uint8 = null; // required // Delay in milliseconds before initiating a restart attempt. delay_before_restart_ms:uint32 = null; // required } diff --git a/score/launch_manager/src/daemon/src/configuration/recovery_action_config.hpp b/score/launch_manager/src/daemon/src/configuration/recovery_action_config.hpp index fb4e19c9bc..b82006e5bd 100644 --- a/score/launch_manager/src/daemon/src/configuration/recovery_action_config.hpp +++ b/score/launch_manager/src/daemon/src/configuration/recovery_action_config.hpp @@ -21,7 +21,7 @@ namespace score::mw::lifecycle::internal::configuration struct RestartAction { - std::uint32_t number_of_attempts{}; + std::uint8_t number_of_attempts{}; std::uint32_t delay_before_restart_ms{}; }; diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.cpp index 2a56adbd45..15f869105d 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.cpp @@ -33,9 +33,10 @@ ProcessInfoNode::ProcessInfoNode(configuration::ComponentConfig&& config, Proces identifier_(config_.name), termination_waiter_(std::make_unique()) { + // number_of_attempts is pre-validated with max value: 254, to avoid integer overflow when incremented if (config_.deployment_config.ready_recovery_action.has_value()) { - start_tries_ = config_.deployment_config.ready_recovery_action->number_of_attempts + 1; + start_tries_ = config_.deployment_config.ready_recovery_action->number_of_attempts + 1U; } const configuration::ApplicationProfile& app_profile = config_.component_properties.application_profile; From bbf86233003e1303ab074c56800704cc3ad7d32e Mon Sep 17 00:00:00 2001 From: Cameron Craig Date: Thu, 15 Oct 2026 09:45:14 +0100 Subject: [PATCH 02/10] test: Config validation for number_of_attempts --- scripts/config_mapping/integration_tests.py | 22 ++++++++++-- .../expected_output/lm_config_gen.json | 2 +- .../full_config_test/input/lm_config.json | 2 +- .../input/invalid_number_of_attempts.json | 36 +++++++++++++++++++ 4 files changed, 58 insertions(+), 4 deletions(-) create mode 100644 scripts/config_mapping/tests/schema_validation_failure_test/input/invalid_number_of_attempts.json diff --git a/scripts/config_mapping/integration_tests.py b/scripts/config_mapping/integration_tests.py index 079a14d65b..bea6490a1a 100644 --- a/scripts/config_mapping/integration_tests.py +++ b/scripts/config_mapping/integration_tests.py @@ -239,14 +239,32 @@ def test_custom_validation_failures( ) -def test_schema_validation_failures(schema_file): +# Define the parameterized test cases +SCHEMA_VALIDATION_TEST_CASES = [ + { + "config_filename": "lm_config.json", + "test_id": "missing_recovery_action", + }, + { + "config_filename": "invalid_number_of_attempts.json", + "test_id": "invalid_number_of_attempts", + }, +] + + +@pytest.mark.parametrize( + "config_filename", + [(case["config_filename"]) for case in SCHEMA_VALIDATION_TEST_CASES], + ids=[case["test_id"] for case in SCHEMA_VALIDATION_TEST_CASES], +) +def test_schema_validation_failures(schema_file, config_filename): """ Test that schema validation errors are correctly raised when the input configuration does not conform to the defined JSON schema. The input configuration contains the following issues: * Missing required field "initial_run_target" """ test_name = "schema_validation_failure_test" - input_file = tests_dir / test_name / "input" / "lm_config.json" + input_file = tests_dir / test_name / "input" / config_filename try: run(input_file, test_name, schema_file) diff --git a/scripts/config_mapping/tests/full_config_test/expected_output/lm_config_gen.json b/scripts/config_mapping/tests/full_config_test/expected_output/lm_config_gen.json index 9490356c18..3a1da6a208 100644 --- a/scripts/config_mapping/tests/full_config_test/expected_output/lm_config_gen.json +++ b/scripts/config_mapping/tests/full_config_test/expected_output/lm_config_gen.json @@ -64,7 +64,7 @@ } ], "ready_recovery_action": { - "number_of_attempts": 5, + "number_of_attempts": 254, "delay_before_restart_ms": 500 }, "recovery_action": { diff --git a/scripts/config_mapping/tests/full_config_test/input/lm_config.json b/scripts/config_mapping/tests/full_config_test/input/lm_config.json index 559a00ca4f..bc9ae729ac 100644 --- a/scripts/config_mapping/tests/full_config_test/input/lm_config.json +++ b/scripts/config_mapping/tests/full_config_test/input/lm_config.json @@ -99,7 +99,7 @@ "working_dir": "/var/run/a", "ready_recovery_action": { "restart": { - "number_of_attempts": 5, + "number_of_attempts": 254, "delay_before_restart_ms": 500 } }, diff --git a/scripts/config_mapping/tests/schema_validation_failure_test/input/invalid_number_of_attempts.json b/scripts/config_mapping/tests/schema_validation_failure_test/input/invalid_number_of_attempts.json new file mode 100644 index 0000000000..aaf74dfa9b --- /dev/null +++ b/scripts/config_mapping/tests/schema_validation_failure_test/input/invalid_number_of_attempts.json @@ -0,0 +1,36 @@ +{ + "schema_version": 1, + "defaults": {}, + "components": { + "reporting_and_kterminated": { + "component_properties": { + "binary_name": "/bin/true", + "ready_condition": { + "process_state": "Terminated" + }, + "application_profile": { + "application_type": "Reporting", + "is_self_terminating": true + } + }, + "deployment_config": { + "ready_recovery_action": { + "restart": { + "number_of_attempts": 255 + } + } + } + } + }, + "run_targets": { + "Startup": { + "depends_on": [ + "reporting_and_kterminated" + ] + } + }, + "initial_run_target": "Startup", + "fallback_run_target": { + "depends_on": [] + } +} From 09d9d5a0237e4c1f090f9cb9f0f82302f856abcc Mon Sep 17 00:00:00 2001 From: Cameron Craig Date: Thu, 15 Oct 2026 10:46:26 +0100 Subject: [PATCH 03/10] test: Add flatbuffer config loader test --- .../details/flatbuffer_config_loader_UT.cpp | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/score/launch_manager/src/daemon/src/configuration/details/flatbuffer_config_loader_UT.cpp b/score/launch_manager/src/daemon/src/configuration/details/flatbuffer_config_loader_UT.cpp index 5a32ea7ff8..6f4a208dba 100644 --- a/score/launch_manager/src/daemon/src/configuration/details/flatbuffer_config_loader_UT.cpp +++ b/score/launch_manager/src/daemon/src/configuration/details/flatbuffer_config_loader_UT.cpp @@ -809,5 +809,78 @@ TEST_F(FlatbufferConfigLoaderTest, MissingSchemaVersionReturnsInvalidFormat) EXPECT_THAT(result.error(), Eq(IConfigLoader::Error::InvalidFormat)); } +// ============================================================================ +// Boundary tests +// ============================================================================ + +struct NumberOfAttemptsParameterizedTestParams +{ + // Using uint_16_t to we can represent values > 255, + // so we can test overflow of uint8_t + std::uint16_t input; + std::uint16_t expected; +}; + +class NumberOfAttemptsParameterizedTest : public FlatbufferConfigLoaderTest, + public ::testing::WithParamInterface +{ +}; + +TEST_P( + NumberOfAttemptsParameterizedTest, + GivenNumberOfAttemptsBoundaryValues_WhenFlatbufferLoaded_ThenNumberOfAttemptsIsValid) +{ + RecordProperty("DerivationTechnique", "boundary-values"); + RecordProperty( + "Description", + "Given upper and lower boundary values for number_of_attempts: [0, 254, 255, 256]. " + "And a valid boundary of 0 to 255 inclusive. " + "When the flatbuffer is loaded. " + "Then values with the boundary are preserved. " + "And values outside the boundary overflow. "); + + // Given + ::flatbuffers::FlatBufferBuilder fbb; + const std::uint16_t NUMBER_OF_ATTEMPTS = GetParam().input; + auto restart = fb::CreateRestartAction(fbb, NUMBER_OF_ATTEMPTS, 1500); + auto bin_dir = fbb.CreateString("/opt"); + auto work_dir = fbb.CreateString("/tmp"); + auto sandbox = buildDefaultSandbox(fbb); + auto deploy = fb::CreateDeploymentConfig( + fbb, + 1000 /*ready_timeout_ms*/, + 1000 /*shutdown_timeout_ms*/, + 0 /*environmental_variables*/, + bin_dir, + work_dir, + restart, + 0 /*recovery_action*/, + sandbox); + + auto comp = buildDefaultComponent(fbb, "restart_comp", buildDefaultComponentProperties(fbb), deploy); + auto comps = fbb.CreateVector(std::vector<::flatbuffers::Offset>{comp}); + + // When + auto result = loadBuffer(buildConfigWithComponents(fbb, comps)); + + // Then + ASSERT_THAT(result.has_value(), IsTrue()); + ASSERT_THAT(result->components().size(), Eq(1U)); + ASSERT_THAT(result->components()[0].deployment_config.ready_recovery_action.has_value(), IsTrue()); + + const auto& ra = result->components()[0].deployment_config.ready_recovery_action.value(); + EXPECT_THAT(ra.number_of_attempts, Eq(GetParam().expected)); +} + +INSTANTIATE_TEST_SUITE_P( + NumberOfAttemptsBoundaryValues, + NumberOfAttemptsParameterizedTest, + ::testing::Values( + NumberOfAttemptsParameterizedTestParams{0U, 0U}, + NumberOfAttemptsParameterizedTestParams{1U, 1U}, + NumberOfAttemptsParameterizedTestParams{254U, 254U}, + NumberOfAttemptsParameterizedTestParams{255U, 255U}, + NumberOfAttemptsParameterizedTestParams{256U, 0U})); + } // namespace } // namespace score::mw::lifecycle::internal::configuration From 3006fe564a7802e21c5bba102829e8317bdbfd50 Mon Sep 17 00:00:00 2001 From: Cameron Craig Date: Fri, 16 Oct 2026 08:49:42 +0100 Subject: [PATCH 04/10] fix: Prevent integer overflow at runtime --- .../details/process_info_node.cpp | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.cpp index 15f869105d..cb96a9aff8 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.cpp @@ -19,6 +19,7 @@ #include "score/mw/launch_manager/process_group_manager/details/safe_process_map.hpp" #include #include +#include #include namespace score::mw::lifecycle::internal @@ -33,10 +34,21 @@ ProcessInfoNode::ProcessInfoNode(configuration::ComponentConfig&& config, Proces identifier_(config_.name), termination_waiter_(std::make_unique()) { - // number_of_attempts is pre-validated with max value: 254, to avoid integer overflow when incremented + // number_of_attempts is pre-validated with max value: 254 + // Warn user and prevent overflow on the below increment if 255 is passed if (config_.deployment_config.ready_recovery_action.has_value()) { - start_tries_ = config_.deployment_config.ready_recovery_action->number_of_attempts + 1U; + auto number_of_attempts = config_.deployment_config.ready_recovery_action->number_of_attempts; + start_tries_ = number_of_attempts; + if (number_of_attempts < UINT8_MAX) + { + start_tries_ += 1U; + } + else + { + LM_LOG_WARN() << "Configured number_of_attempts (" << number_of_attempts << ") is too large, using " + << (number_of_attempts - 1U) << " instead."; + } } const configuration::ApplicationProfile& app_profile = config_.component_properties.application_profile; From b202a4a03c917cc34165f402a35888272c62fb53 Mon Sep 17 00:00:00 2001 From: Cameron Craig Date: Fri, 16 Oct 2026 09:45:46 +0100 Subject: [PATCH 05/10] test: Add process_info_node test --- .../details/process_info_node_UT.cpp | 71 ++++++++++++++++++- 1 file changed, 69 insertions(+), 2 deletions(-) diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node_UT.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node_UT.cpp index bee6f9c9ad..cce1078324 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node_UT.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node_UT.cpp @@ -84,7 +84,7 @@ class ProcessInfoNodeFixture : public ::testing::Test /// @note ComponentConfig is move-only, so a fresh one is built for every node. std::unique_ptr createProcessInfoNode( configuration::ApplicationType application_type = configuration::ApplicationType::Reporting, - std::uint32_t restart_attempts = 0U, + std::uint8_t restart_attempts = 0U, bool self_terminating = false, configuration::ProcessState ready_state = configuration::ProcessState::Running) { @@ -135,7 +135,7 @@ class ProcessInfoNodeFixture : public ::testing::Test /// @brief Helper method to create a ProcessInfoNode that is self-terminating. std::unique_ptr createSelfTerminatingProcessInfoNode( configuration::ApplicationType application_type = configuration::ApplicationType::Reporting, - std::uint32_t restart_attempts = 0U) + std::uint8_t restart_attempts = 0U) { return createProcessInfoNode(application_type, restart_attempts, true); } @@ -654,6 +654,73 @@ TEST_F(ProcessInfoNodeStartupCrashTest, TimeoutThenSuccess_WithRestarts) ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kRunning)); } +struct NumberOfAttemptsParameterizedTestParams +{ + // Using uint_16_t to we can represent values > 255, + // so we can test overflow of uint8_t + std::uint16_t input; + std::uint16_t expected_attempts; +}; + +class NumberOfAttemptsParameterizedTest : public ProcessInfoNodeStartupCrashTest, + public ::testing::WithParamInterface +{ +}; + +TEST_P( + NumberOfAttemptsParameterizedTest, + GivenBoundaryValuesForNumberOfAttempts_ExpectCrashBeforeReady_WhenActivateCalled) +{ + RecordProperty("DerivationTechnique", "boundary-values"); + RecordProperty( + "Description", + "Given boundary values for number_of_attempts: [0, 1, 254, 255]. " + "And a valid boundary of 0 to 254 inclusive. " + "Expect that the OS handler detects a crash on every attempt. " + "Expect that the number of start attempts equals the expected values: [1, 2, 255, 255]. " + "When activate() is called. " + "Then kErrorBeforeReady is returned for all number_of_attempts values. "); + + // Expect + expectActivationReport(0); + const std::uint16_t kRestartAttempts = GetParam().input; + const std::uint16_t kExpectedTotalAttempts = GetParam().expected_attempts; + auto node = createProcessInfoNode(configuration::ApplicationType::ReportingAndSupervised, kRestartAttempts); + + EXPECT_CALL(mock_processIf_, startProcess(_, _, _)) + .Times(kExpectedTotalAttempts) + .WillRepeatedly(Return(osal::OsalReturnType::kSuccess)); + EXPECT_CALL(*process_map_, insertIfNotTerminated(_, _)) + .Times(kExpectedTotalAttempts) + .WillRepeatedly(Return(score::mw::lifecycle::internal::SafeProcessMapReturnType::kOk)); + // Simulate the OS handler detecting the crash on every attempt, while the process is still waiting to reach + // kRunning. + EXPECT_CALL(mock_processIf_, waitForkRunning(_, _)) + .Times(kExpectedTotalAttempts) + .WillRepeatedly(DoAll( + InvokeWithoutArgs([node = node.get()] { + static_cast(node->tryHandleTermination(-1)); + }), + Return(osal::OsalReturnType::kFail))); + + // When + auto result = node->activate(score::cpp::stop_token{}); + + // Then + ASSERT_THAT(result.has_value(), IsFalse()); + ASSERT_THAT(result.error(), Eq(IComponent::ComponentError::kErrorBeforeReady)); + ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kTerminated)); +} + +INSTANTIATE_TEST_SUITE_P( + NumberOfAttemptsBoundaryValues, + NumberOfAttemptsParameterizedTest, + ::testing::Values( + NumberOfAttemptsParameterizedTestParams{0U, 1U}, + NumberOfAttemptsParameterizedTestParams{1U, 2U}, + NumberOfAttemptsParameterizedTestParams{254U, 255U}, + NumberOfAttemptsParameterizedTestParams{255U, 255U})); + // Bundles unexpected terminations that occur after the ready condition has been reached. class ProcessInfoNodeUnexpectedTerminationTest : public ProcessInfoNodeFixture { From cc5d608390d54543a2702b9e788ba95ec9314329 Mon Sep 17 00:00:00 2001 From: Cameron Craig Date: Fri, 16 Oct 2026 10:49:25 +0100 Subject: [PATCH 06/10] chore: Improve test spec --- .../src/configuration/details/flatbuffer_config_loader_UT.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/score/launch_manager/src/daemon/src/configuration/details/flatbuffer_config_loader_UT.cpp b/score/launch_manager/src/daemon/src/configuration/details/flatbuffer_config_loader_UT.cpp index 6f4a208dba..9d7270bd7d 100644 --- a/score/launch_manager/src/daemon/src/configuration/details/flatbuffer_config_loader_UT.cpp +++ b/score/launch_manager/src/daemon/src/configuration/details/flatbuffer_config_loader_UT.cpp @@ -833,10 +833,10 @@ TEST_P( RecordProperty("DerivationTechnique", "boundary-values"); RecordProperty( "Description", - "Given upper and lower boundary values for number_of_attempts: [0, 254, 255, 256]. " + "Given upper and lower boundary values for number_of_attempts: [0, 1, 254, 255, 256]. " "And a valid boundary of 0 to 255 inclusive. " "When the flatbuffer is loaded. " - "Then values with the boundary are preserved. " + "Then values within the boundary are preserved. " "And values outside the boundary overflow. "); // Given From a0049e1537cde77de3cd4c8012c8c39572768acd Mon Sep 17 00:00:00 2001 From: Cameron Craig Date: Mon, 19 Oct 2026 10:42:56 +0100 Subject: [PATCH 07/10] refactor: Allow 0 to 255 retries --- .../docs/product_documentation/configuration.rst | 2 +- .../config_schema/launch_manager.schema.json | 2 +- .../details/process_info_node.cpp | 14 ++------------ .../details/process_info_node.hpp | 4 ++-- .../details/process_info_node_UT.cpp | 4 ++-- 5 files changed, 8 insertions(+), 18 deletions(-) diff --git a/score/launch_manager/docs/product_documentation/configuration.rst b/score/launch_manager/docs/product_documentation/configuration.rst index dbef803815..c94f6610c2 100644 --- a/score/launch_manager/docs/product_documentation/configuration.rst +++ b/score/launch_manager/docs/product_documentation/configuration.rst @@ -129,7 +129,7 @@ restart (object, optional) restart.number_of_attempts (integer, optional) Description: Specifies the maximum number of restart attempts before the **Launch Manager** concludes that recovery cannot succeed for the component. - Constraint: Must be between 0 and 254 inclusive. + Constraint: Must be between 0 and 255 inclusive. restart.delay_before_restart_ms (integer, optional) Description: Specifies the delay duration, in milliseconds, that the **Launch Manager** waits before initiating a restart attempt. diff --git a/score/launch_manager/src/daemon/src/configuration/config_schema/launch_manager.schema.json b/score/launch_manager/src/daemon/src/configuration/config_schema/launch_manager.schema.json index 926ada9f70..eaf351b48d 100644 --- a/score/launch_manager/src/daemon/src/configuration/config_schema/launch_manager.schema.json +++ b/score/launch_manager/src/daemon/src/configuration/config_schema/launch_manager.schema.json @@ -150,7 +150,7 @@ "number_of_attempts": { "type": "integer", "minimum": 0, - "maximum": 254, + "maximum": 255, "description": "Specifies the maximum number of restart attempts before the Launch Manager concludes that recovery cannot succeed." }, "delay_before_restart_ms": { diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.cpp index cb96a9aff8..79bff3f6a6 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.cpp @@ -38,17 +38,7 @@ ProcessInfoNode::ProcessInfoNode(configuration::ComponentConfig&& config, Proces // Warn user and prevent overflow on the below increment if 255 is passed if (config_.deployment_config.ready_recovery_action.has_value()) { - auto number_of_attempts = config_.deployment_config.ready_recovery_action->number_of_attempts; - start_tries_ = number_of_attempts; - if (number_of_attempts < UINT8_MAX) - { - start_tries_ += 1U; - } - else - { - LM_LOG_WARN() << "Configured number_of_attempts (" << number_of_attempts << ") is too large, using " - << (number_of_attempts - 1U) << " instead."; - } + start_retries_ = config_.deployment_config.ready_recovery_action->number_of_attempts; } const configuration::ApplicationProfile& app_profile = config_.component_properties.application_profile; @@ -255,7 +245,7 @@ IComponent::RequestResult ProcessInfoNode::startProcess(score::cpp::stop_token s std::optional error; const std::chrono::time_point initial_time = std::chrono::steady_clock::now(); - for (std::uint8_t attempts = start_tries_; attempts != 0U; attempts--) + for (std::uint16_t attempts = 0U; attempts <= start_retries_; attempts++) { // setState(kIdle) will fail if the state is: // - Starting: this would mean we did not set state to kFailed on failure or exit when we successfully launched diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.hpp index 6fbb30123c..9c6b87538c 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.hpp @@ -205,8 +205,8 @@ class ProcessInfoNode final : public IComponent /// @brief Interface for managing the process's alive supervision. std::unique_ptr supervision_handle_; - /// @brief Number ot times to try run the process. - std::uint8_t start_tries_{1U}; + /// @brief Number of times to retry starting the process. + std::uint8_t start_retries_{0U}; /// @brief Unique hash to identify this node. IdentifierHash identifier_; diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node_UT.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node_UT.cpp index cce1078324..d51f42abe2 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node_UT.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node_UT.cpp @@ -677,7 +677,7 @@ TEST_P( "Given boundary values for number_of_attempts: [0, 1, 254, 255]. " "And a valid boundary of 0 to 254 inclusive. " "Expect that the OS handler detects a crash on every attempt. " - "Expect that the number of start attempts equals the expected values: [1, 2, 255, 255]. " + "Expect that the number of start attempts equals the expected values: [1, 2, 255, 256]. " "When activate() is called. " "Then kErrorBeforeReady is returned for all number_of_attempts values. "); @@ -719,7 +719,7 @@ INSTANTIATE_TEST_SUITE_P( NumberOfAttemptsParameterizedTestParams{0U, 1U}, NumberOfAttemptsParameterizedTestParams{1U, 2U}, NumberOfAttemptsParameterizedTestParams{254U, 255U}, - NumberOfAttemptsParameterizedTestParams{255U, 255U})); + NumberOfAttemptsParameterizedTestParams{255U, 256U})); // Bundles unexpected terminations that occur after the ready condition has been reached. class ProcessInfoNodeUnexpectedTerminationTest : public ProcessInfoNodeFixture From 1205d2ea4f2722e47e12e96fbcdabcdf431de384 Mon Sep 17 00:00:00 2001 From: Cameron Craig Date: Mon, 19 Oct 2026 12:11:15 +0100 Subject: [PATCH 08/10] test: Increment invalid number_of_attempts --- .../input/invalid_number_of_attempts.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/config_mapping/tests/schema_validation_failure_test/input/invalid_number_of_attempts.json b/scripts/config_mapping/tests/schema_validation_failure_test/input/invalid_number_of_attempts.json index aaf74dfa9b..a12d64c211 100644 --- a/scripts/config_mapping/tests/schema_validation_failure_test/input/invalid_number_of_attempts.json +++ b/scripts/config_mapping/tests/schema_validation_failure_test/input/invalid_number_of_attempts.json @@ -16,7 +16,7 @@ "deployment_config": { "ready_recovery_action": { "restart": { - "number_of_attempts": 255 + "number_of_attempts": 256 } } } From a2c675eedea8d71a6d2ed9d31187133ddd3bd4c5 Mon Sep 17 00:00:00 2001 From: Cameron Craig Date: Mon, 19 Oct 2026 12:32:28 +0100 Subject: [PATCH 09/10] fix: Move ctor --- .../src/process_group_manager/details/process_info_node.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.hpp index 9c6b87538c..298fc942c5 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.hpp @@ -68,7 +68,7 @@ class ProcessInfoNode final : public IComponent sync_(std::move(other.sync_)), process_handling_(std::move(other.process_handling_)), supervision_handle_(std::move(other.supervision_handle_)), - start_tries_(other.start_tries_), + start_retries_(other.start_retries_), identifier_(other.identifier_), termination_result_(other.termination_result_), termination_waiter_(std::move(other.termination_waiter_)) From b1887871f81a786dea478de10a6155a115fe1be3 Mon Sep 17 00:00:00 2001 From: Cameron Craig Date: Mon, 19 Oct 2026 13:24:16 +0100 Subject: [PATCH 10/10] docs: Remove out of date comments --- .../src/process_group_manager/details/process_info_node.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.cpp index 79bff3f6a6..138f68892f 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.cpp @@ -34,8 +34,6 @@ ProcessInfoNode::ProcessInfoNode(configuration::ComponentConfig&& config, Proces identifier_(config_.name), termination_waiter_(std::make_unique()) { - // number_of_attempts is pre-validated with max value: 254 - // Warn user and prevent overflow on the below increment if 255 is passed if (config_.deployment_config.ready_recovery_action.has_value()) { start_retries_ = config_.deployment_config.ready_recovery_action->number_of_attempts;