From c9c994cf87c4ab296171a0475050c699f03a1e22 Mon Sep 17 00:00:00 2001 From: mhossamr Date: Thu, 24 Sep 2026 18:26:27 +0300 Subject: [PATCH 1/3] fix(launch_manager): remove allocation in ProcessGroupManager Replace heap-allocating InterruptiblePromise/Future in getActiveRunTarget() and setRequestedRunTarget() with a stack-allocated CompletionSlot backed by score::concurrency::Notification, ensuring zero heap allocations at runtime. Signed-off-by: mhossamr --- .../daemon/src/process_group_manager/BUILD | 2 +- .../src/process_group_manager/details/BUILD | 26 ++- .../details/completion_slot.hpp | 72 ++++++++ .../details/completion_slot_UT.cpp | 172 ++++++++++++++++++ .../details/component_event.hpp | 7 +- .../details/component_event_queue.hpp | 7 +- .../details/component_event_queue_UT.cpp | 39 ++++ .../details/process_group_manager_UT.cpp | 50 +++++ .../process_group_manager.cpp | 45 ++--- 9 files changed, 391 insertions(+), 29 deletions(-) create mode 100644 score/launch_manager/src/daemon/src/process_group_manager/details/completion_slot.hpp create mode 100644 score/launch_manager/src/daemon/src/process_group_manager/details/completion_slot_UT.cpp diff --git a/score/launch_manager/src/daemon/src/process_group_manager/BUILD b/score/launch_manager/src/daemon/src/process_group_manager/BUILD index 319bd02da0..61f8c63bfc 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/BUILD +++ b/score/launch_manager/src/daemon/src/process_group_manager/BUILD @@ -55,6 +55,7 @@ cc_library( "//score/launch_manager/src/daemon/src/configuration:config", "//score/launch_manager/src/daemon/src/osal:ipc_comms", "//score/launch_manager/src/daemon/src/osal:wait_for_file", + "//score/launch_manager/src/daemon/src/process_group_manager/details:completion_slot", "//score/launch_manager/src/daemon/src/process_group_manager/details:component_event", "//score/launch_manager/src/daemon/src/process_group_manager/details:graph", "//score/launch_manager/src/daemon/src/process_group_manager/details:os_handler", @@ -65,7 +66,6 @@ cc_library( "//score/launch_manager/src/daemon/src/recovery_client", "//score/launch_manager/src/daemon/src/watchdog:i_watchdog_if", "//score/launch_manager/src/lm_control", - "@score_baselibs//score/concurrency/future", "@score_baselibs//score/language/futurecpp", ], ) diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/BUILD b/score/launch_manager/src/daemon/src/process_group_manager/details/BUILD index 08b1f22db0..d94464c3d8 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/BUILD +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/BUILD @@ -88,6 +88,18 @@ cc_library( ], ) +cc_library( + name = "completion_slot", + hdrs = ["completion_slot.hpp"], + include_prefix = "score/mw/launch_manager/process_group_manager/details", + strip_include_prefix = "/score/launch_manager/src/daemon/src/process_group_manager/details", + visibility = ["//score/launch_manager/src/daemon/src/process_group_manager:__subpackages__"], + deps = [ + "@score_baselibs//score/concurrency:notification", + "@score_baselibs//score/language/futurecpp", + ], +) + cc_library( name = "component_event", hdrs = ["component_event.hpp"], @@ -95,9 +107,10 @@ cc_library( strip_include_prefix = "/score/launch_manager/src/daemon/src/process_group_manager/details", visibility = ["//score/launch_manager/src/daemon/src/process_group_manager:__pkg__"], deps = [ + ":completion_slot", ":icomponent", "//score/launch_manager/src/daemon/src/common:identifier_hash", - "@score_baselibs//score/concurrency/future", + "@score_baselibs//score/result", ], ) @@ -128,6 +141,17 @@ cc_library( ], ) +lm_cc_test( + name = "completion_slot_UT", + srcs = ["completion_slot_UT.cpp"], + deps = [ + ":completion_slot", + "//score/launch_manager/src/daemon/src/common:identifier_hash", + "@googletest//:gtest_main", + "@score_baselibs//score/result", + ], +) + lm_cc_test( name = "component_event_queue_UT", srcs = ["component_event_queue_UT.cpp"], diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/completion_slot.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/completion_slot.hpp new file mode 100644 index 0000000000..f2e0c21703 --- /dev/null +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/completion_slot.hpp @@ -0,0 +1,72 @@ +/******************************************************************************** + * 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_LCM_COMPLETION_SLOT_HPP_INCLUDED +#define SCORE_LCM_COMPLETION_SLOT_HPP_INCLUDED + +#include "score/concurrency/notification.h" +#include "score/stop_token.hpp" + +#include +#include + +namespace score::mw::lifecycle::internal +{ + +/// @brief A non-allocating, single-producer, single-consumer one-shot completion slot. +/// +/// Designed to be allocated on the caller's stack frame. The caller passes a pointer +/// to the slot via an event to the main event loop thread and waits for the result. +/// This completely avoids dynamic memory allocation in contrast to std::promise/future +/// or InterruptiblePromise/Future. +template +class CompletionSlot final +{ + public: + CompletionSlot() = default; + ~CompletionSlot() = default; + CompletionSlot(const CompletionSlot&) = delete; + CompletionSlot& operator=(const CompletionSlot&) = delete; + CompletionSlot(CompletionSlot&&) = delete; + CompletionSlot& operator=(CompletionSlot&&) = delete; + + /// @brief Publishes the result and wakes the waiting thread. Called once, typically by the main thread. + void complete(T value) + { + value_ = std::move(value); + done_.notify(); + } + + /// @brief Blocks until complete() has run or interruption is requested via token. + /// @param token A stop_token that can abort the wait. + /// @return The completed value, or std::nullopt if the wait was aborted. + [[nodiscard]] std::optional wait(score::cpp::stop_token token = {}) + { + if (!done_.waitWithAbort(token)) + { + return std::nullopt; + } + return std::move(value_); + } + + private: + /// @brief Inline storage for the result; no dynamic memory allocation. + std::optional value_{}; + + /// @brief Notification held by value (mutex + cv + flag). No dynamic memory allocation. + score::concurrency::Notification done_{}; +}; + +} // namespace score::mw::lifecycle::internal + +#endif // SCORE_LCM_COMPLETION_SLOT_HPP_INCLUDED diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/completion_slot_UT.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/completion_slot_UT.cpp new file mode 100644 index 0000000000..ce099770bf --- /dev/null +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/completion_slot_UT.cpp @@ -0,0 +1,172 @@ +/******************************************************************************** + * 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 "score/mw/launch_manager/process_group_manager/details/completion_slot.hpp" + +#include + +#include +#include +#include +#include +#include + +#include "score/mw/launch_manager/common/identifier_hash.hpp" +#include "score/result/result.h" + +namespace +{ + +thread_local bool g_tracking_enabled = false; +thread_local std::size_t g_tracked_allocations = 0; + +} // namespace + +void* operator new(std::size_t size) +{ + if (g_tracking_enabled) + { + ++g_tracked_allocations; + } + void* ptr = std::malloc(size); + if (ptr == nullptr) + { + throw std::bad_alloc(); + } + return ptr; +} + +void operator delete(void* ptr) noexcept +{ + std::free(ptr); +} + +void operator delete(void* ptr, std::size_t) noexcept +{ + std::free(ptr); +} + +namespace score::mw::lifecycle::internal +{ +namespace +{ + +class CompletionSlotTest : public ::testing::Test +{ + protected: + void SetUp() override + { + RecordProperty("TestType", "unit-test"); + RecordProperty("DerivationTechnique", "requirements-based"); + } +}; + +TEST_F(CompletionSlotTest, SynchronousCompleteThenWaitReturnsValue) +{ + RecordProperty("Description", "Verify CompletionSlot returns the published value synchronously."); + + CompletionSlot slot; + slot.complete(42); + + auto result = slot.wait(); + ASSERT_TRUE(result.has_value()); + EXPECT_EQ(result.value(), 42); +} + +TEST_F(CompletionSlotTest, SynchronousCompleteResultVoid) +{ + RecordProperty("Description", "Verify CompletionSlot works with score::Result."); + + CompletionSlot> slot; + slot.complete(score::Result{}); + + auto result = slot.wait(); + ASSERT_TRUE(result.has_value()); + EXPECT_TRUE(result.value().has_value()); +} + +TEST_F(CompletionSlotTest, AsynchronousCompleteFromWorkerThread) +{ + RecordProperty("Description", "Verify CompletionSlot unblocks waiting thread when completed asynchronously."); + + CompletionSlot> slot; + const IdentifierHash expected_hash{"TargetRunState"}; + + std::thread worker([&slot, expected_hash]() { + std::this_thread::sleep_for(std::chrono::milliseconds{10}); + slot.complete(score::Result{expected_hash}); + }); + + auto result = slot.wait(); + worker.join(); + + ASSERT_TRUE(result.has_value()); + ASSERT_TRUE(result.value().has_value()); + EXPECT_EQ(result.value().value(), expected_hash); +} + +TEST_F(CompletionSlotTest, PreCancelledStopTokenAbortsWait) +{ + RecordProperty("Description", "Verify CompletionSlot::wait returns nullopt if stop_token is already stopped."); + + CompletionSlot slot; + score::cpp::stop_source source; + source.request_stop(); + + auto result = slot.wait(source.get_token()); + EXPECT_FALSE(result.has_value()); +} + +TEST_F(CompletionSlotTest, ZeroAllocationInCompleteAndWait) +{ + RecordProperty("Description", "Verify CompletionSlot complete() and wait() perform zero dynamic memory allocations."); + + const IdentifierHash test_hash{"Running"}; + + g_tracked_allocations = 0; + g_tracking_enabled = true; + + { + CompletionSlot> slot; + slot.complete(score::Result{test_hash}); + auto result = slot.wait(); + ASSERT_TRUE(result.has_value()); + ASSERT_TRUE(result.value().has_value()); + EXPECT_EQ(result.value().value(), test_hash); + } + + g_tracking_enabled = false; + EXPECT_EQ(g_tracked_allocations, 0U); +} + +TEST_F(CompletionSlotTest, ZeroAllocationInResultVoidCompleteAndWait) +{ + RecordProperty("Description", "Verify CompletionSlot with Result performs zero dynamic memory allocations."); + + g_tracked_allocations = 0; + g_tracking_enabled = true; + + { + CompletionSlot> slot; + slot.complete(score::Result{}); + auto result = slot.wait(); + ASSERT_TRUE(result.has_value()); + EXPECT_TRUE(result.value().has_value()); + } + + g_tracking_enabled = false; + EXPECT_EQ(g_tracked_allocations, 0U); +} + +} // namespace +} // namespace score::mw::lifecycle::internal diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/component_event.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/component_event.hpp index 7ac3d0efd9..63ab852520 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/component_event.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/component_event.hpp @@ -17,9 +17,10 @@ #include #include -#include "score/concurrency/future/interruptible_promise.h" #include "score/mw/launch_manager/common/identifier_hash.hpp" +#include "score/mw/launch_manager/process_group_manager/details/completion_slot.hpp" #include "score/mw/launch_manager/process_group_manager/details/icomponent.hpp" +#include "score/result/result.h" namespace score::mw::lifecycle::internal { @@ -73,14 +74,14 @@ struct [[nodiscard]] SupervisionFailure /// @brief The state manager wants to know which run target is currently active. struct [[nodiscard]] GetActiveRunTarget { - concurrency::InterruptiblePromise> promise; + CompletionSlot>* completion{nullptr}; }; /// @brief The state manager wants to change to a different run target. struct [[nodiscard]] SetRequestedRunTarget { IdentifierHash run_target; - concurrency::InterruptiblePromise> promise; + CompletionSlot>* completion{nullptr}; }; /// @brief A graph-relevant state change. There is only ever a single graph, so no process-group diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/component_event_queue.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/component_event_queue.hpp index 66936a2c87..19ec4f4d91 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/component_event_queue.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/component_event_queue.hpp @@ -51,9 +51,12 @@ class ComponentEventQueue final : public IComponentEventPublisherConsumer [[nodiscard]] bool push(ComponentEvent&& event) override { auto result = queue_.push(std::move(event)); - if (!result.has_value() && result.error() == mw::lifecycle::internal::ConcurrencyErrc::kOverflow) + if (!result.has_value()) { - overflow_.store(true, std::memory_order_release); + if (result.error() == mw::lifecycle::internal::ConcurrencyErrc::kOverflow) + { + overflow_.store(true, std::memory_order_release); + } return false; } return true; diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/component_event_queue_UT.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/component_event_queue_UT.cpp index ee29c164ee..3a8eb3b8f3 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/component_event_queue_UT.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/component_event_queue_UT.cpp @@ -80,6 +80,38 @@ TEST_F(ComponentEventQueueTest, GetNextEventReturnsSupervisionFailureWithPayload EXPECT_EQ(failure.process_identifier, process_identifier); } +TEST_F(ComponentEventQueueTest, GetNextEventReturnsGetActiveRunTargetWithCompletionSlot) +{ + RecordProperty( + "Description", + "Verify a pushed GetActiveRunTarget event is returned with its CompletionSlot pointer intact."); + CompletionSlot> slot; + EXPECT_TRUE(queue_.push(GetActiveRunTarget{completion : &slot})); + + auto event = queue_.getNextEvent(); + ASSERT_TRUE(event.has_value()); + ASSERT_TRUE(std::holds_alternative(*event)); + const auto& get_active = std::get(*event); + EXPECT_EQ(get_active.completion, &slot); +} + +TEST_F(ComponentEventQueueTest, GetNextEventReturnsSetRequestedRunTargetWithPayloadAndCompletionSlot) +{ + RecordProperty( + "Description", + "Verify a pushed SetRequestedRunTarget event is returned with its run_target and CompletionSlot pointer intact."); + CompletionSlot> slot; + const IdentifierHash target{"RunTargetA"}; + EXPECT_TRUE(queue_.push(SetRequestedRunTarget{run_target : target, completion : &slot})); + + auto event = queue_.getNextEvent(); + ASSERT_TRUE(event.has_value()); + ASSERT_TRUE(std::holds_alternative(*event)); + const auto& set_target = std::get(*event); + EXPECT_EQ(set_target.run_target, target); + EXPECT_EQ(set_target.completion, &slot); +} + TEST_F(ComponentEventQueueTest, GetOverflowStaysFalseUnderNormalUsage) { RecordProperty("Description", "Verify getOverflow() stays false when events are pushed and drained normally."); @@ -132,4 +164,11 @@ TEST_F(ComponentEventQueueTest, GetNextEventStillDrainsQueuedEventsAfterStop) EXPECT_FALSE(queue_.getNextEvent().has_value()); } +TEST_F(ComponentEventQueueTest, PushReturnsFalseAfterStop) +{ + RecordProperty("Description", "Verify push returns false immediately if the queue has been stopped."); + queue_.stop(); + EXPECT_FALSE(queue_.push(ActivationSuccessful{IdentifierHash{"process"}})); +} + } // namespace score::mw::lifecycle::internal diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/process_group_manager_UT.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/process_group_manager_UT.cpp index 2e76a971bb..a5126411d9 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/process_group_manager_UT.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/process_group_manager_UT.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #include #include @@ -257,5 +258,54 @@ TEST_F(ProcessGroupManagerWatchdogTest, GivenMinimalConfig_ExpectWatchdogDisable process_group_manager_->deinitialize(); } +TEST_F(ProcessGroupManagerWatchdogTest, GetActiveRunTargetAndSetRequestedRunTargetDuringRun) +{ + RecordProperty( + "Description", + "Verify getActiveRunTarget and setRequestedRunTarget complete correctly via event queue during run()."); + expectNormalStartup(); + EXPECT_CALL(*watchdog_, serviceWatchdog()).Times(AtLeast(1)); + EXPECT_CALL(*watchdog_, disable()).Times(1); + EXPECT_CALL(*alive_monitor_, stopMonitoring()).Times(1); + + ASSERT_TRUE(process_group_manager_->initialize()); + + std::thread run_thread([this]() { + process_group_manager_->run(); + }); + + auto active_result = process_group_manager_->getActiveRunTarget(); + EXPECT_FALSE(active_result.has_value()); + EXPECT_EQ(active_result.error(), ExecErrc::kActivationInProgress); + + auto set_result = process_group_manager_->setRequestedRunTarget(IdentifierHash{"InvalidTarget"}); + EXPECT_FALSE(set_result.has_value()); + EXPECT_EQ(set_result.error(), ExecErrc::kRunTargetDoesntExist); + + process_group_manager_->cancel(); + run_thread.join(); +} + +TEST_F(ProcessGroupManagerWatchdogTest, GetActiveRunTargetWhenStoppedReturnsFailedErrorPromptly) +{ + RecordProperty( + "Description", + "Verify getActiveRunTarget and setRequestedRunTarget return error promptly without deadlock when stopped."); + expectNormalStartup(); + EXPECT_CALL(*watchdog_, disable()).Times(2); + EXPECT_CALL(*alive_monitor_, stopMonitoring()).Times(2); + + ASSERT_TRUE(process_group_manager_->initialize()); + process_group_manager_->deinitialize(); + + auto active_result = process_group_manager_->getActiveRunTarget(); + EXPECT_FALSE(active_result.has_value()); + EXPECT_EQ(active_result.error(), ExecErrc::kFailed); + + auto set_result = process_group_manager_->setRequestedRunTarget(IdentifierHash{"Startup"}); + EXPECT_FALSE(set_result.has_value()); + EXPECT_EQ(set_result.error(), ExecErrc::kFailed); +} + } // namespace } // namespace score::mw::lifecycle::internal 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 619e9fb6b6..3d8375aedd 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 @@ -17,9 +17,8 @@ #include #include -#include "score/concurrency/future/interruptible_future.h" -#include "score/concurrency/future/interruptible_promise.h" #include "score/mw/launch_manager/common/log.hpp" +#include "score/mw/launch_manager/process_group_manager/details/completion_slot.hpp" #include "score/mw/launch_manager/process_group_manager/details/component_event.hpp" #include "score/mw/launch_manager/process_group_manager/details/process_monitor.hpp" #include "score/mw/launch_manager/process_group_manager/process_group_manager.hpp" @@ -451,22 +450,23 @@ void ProcessGroupManager::handleGetActiveRunTarget(GetActiveRunTarget* event) co break; } - const auto set_result = event->promise.SetValue(result); - SCORE_LANGUAGE_FUTURECPP_ASSERT(set_result.has_value()); + if (event->completion != nullptr) + { + event->completion->complete(result); + } } Result ProcessGroupManager::getActiveRunTarget() const noexcept { - auto promise = concurrency::InterruptiblePromise>{}; - - auto future_result = promise.GetInterruptibleFuture(); - SCORE_LANGUAGE_FUTURECPP_ASSERT(future_result.has_value()); - auto future = std::move(future_result).value(); + CompletionSlot> slot; - const bool push_result = event_queue_->push(GetActiveRunTarget{promise : std::move(promise)}); - SCORE_LANGUAGE_FUTURECPP_ASSERT(push_result); + const bool push_result = event_queue_->push(GetActiveRunTarget{completion : &slot}); + if (!push_result) + { + return score::MakeUnexpected(ExecErrc::kFailed); + } - const auto get_result = future.Get(cpp::stop_token{}); + const auto get_result = slot.wait(cpp::stop_token{}); SCORE_LANGUAGE_FUTURECPP_ASSERT(get_result.has_value()); return get_result.value(); } @@ -507,22 +507,23 @@ void ProcessGroupManager::handleSetRequestedRunTarget(SetRequestedRunTarget* eve graph_->setRequestStartTime(); } - const auto set_result = event->promise.SetValue(response); - SCORE_LANGUAGE_FUTURECPP_ASSERT(set_result.has_value()); + if (event->completion != nullptr) + { + event->completion->complete(response); + } } Result ProcessGroupManager::setRequestedRunTarget(IdentifierHash run_target) noexcept { - auto promise = concurrency::InterruptiblePromise>{}; - - auto future_result = promise.GetInterruptibleFuture(); - SCORE_LANGUAGE_FUTURECPP_ASSERT(future_result.has_value()); - auto future = std::move(future_result).value(); + CompletionSlot> slot; - const bool push_result = event_queue_->push(SetRequestedRunTarget{run_target, promise : std::move(promise)}); - SCORE_LANGUAGE_FUTURECPP_ASSERT(push_result); + const bool push_result = event_queue_->push(SetRequestedRunTarget{run_target, completion : &slot}); + if (!push_result) + { + return score::MakeUnexpected(ExecErrc::kFailed); + } - const auto get_result = future.Get(cpp::stop_token{}); + const auto get_result = slot.wait(cpp::stop_token{}); SCORE_LANGUAGE_FUTURECPP_ASSERT(get_result.has_value()); return get_result.value(); } From b5bf46fa3e388b75697c20170f5aa1886c772efa Mon Sep 17 00:00:00 2001 From: mhossamr Date: Sun, 27 Sep 2026 11:43:54 +0300 Subject: [PATCH 2/3] refactor(launch_manager): drop stop_token from CompletionSlot::wait() Wait unconditionally to avoid potential use-after-free on caller stack. Signed-off-by: mhossamr --- .../details/completion_slot.hpp | 23 +++++++-------- .../details/completion_slot_UT.cpp | 29 ++++--------------- .../process_group_manager.cpp | 8 ++--- 3 files changed, 18 insertions(+), 42 deletions(-) diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/completion_slot.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/completion_slot.hpp index f2e0c21703..ed74bc2c4a 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/completion_slot.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/completion_slot.hpp @@ -25,10 +25,11 @@ namespace score::mw::lifecycle::internal /// @brief A non-allocating, single-producer, single-consumer one-shot completion slot. /// -/// Designed to be allocated on the caller's stack frame. The caller passes a pointer -/// to the slot via an event to the main event loop thread and waits for the result. -/// This completely avoids dynamic memory allocation in contrast to std::promise/future -/// or InterruptiblePromise/Future. +/// Designed to be allocated on the caller's stack frame. The caller passes a non-owning +/// pointer to the slot via an event to the main event loop thread and blocks unconditionally +/// in wait() until the consumer completes it. This guarantees the slot remains valid on the +/// stack for the duration of the operation, avoiding dynamic memory allocation without +/// exposing unsafe cancellation that could lead to a use-after-free. template class CompletionSlot final { @@ -47,16 +48,12 @@ class CompletionSlot final done_.notify(); } - /// @brief Blocks until complete() has run or interruption is requested via token. - /// @param token A stop_token that can abort the wait. - /// @return The completed value, or std::nullopt if the wait was aborted. - [[nodiscard]] std::optional wait(score::cpp::stop_token token = {}) + /// @brief Blocks unconditionally until complete() has run. Called once, by the owning thread. + /// @return The completed value. + [[nodiscard]] T wait() { - if (!done_.waitWithAbort(token)) - { - return std::nullopt; - } - return std::move(value_); + static_cast(done_.waitWithAbort(score::cpp::stop_token{})); + return std::move(*value_); } private: diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/completion_slot_UT.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/completion_slot_UT.cpp index ce099770bf..891b13fc4e 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/completion_slot_UT.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/completion_slot_UT.cpp @@ -78,9 +78,8 @@ TEST_F(CompletionSlotTest, SynchronousCompleteThenWaitReturnsValue) CompletionSlot slot; slot.complete(42); - auto result = slot.wait(); - ASSERT_TRUE(result.has_value()); - EXPECT_EQ(result.value(), 42); + int result = slot.wait(); + EXPECT_EQ(result, 42); } TEST_F(CompletionSlotTest, SynchronousCompleteResultVoid) @@ -91,8 +90,7 @@ TEST_F(CompletionSlotTest, SynchronousCompleteResultVoid) slot.complete(score::Result{}); auto result = slot.wait(); - ASSERT_TRUE(result.has_value()); - EXPECT_TRUE(result.value().has_value()); + EXPECT_TRUE(result.has_value()); } TEST_F(CompletionSlotTest, AsynchronousCompleteFromWorkerThread) @@ -111,20 +109,7 @@ TEST_F(CompletionSlotTest, AsynchronousCompleteFromWorkerThread) worker.join(); ASSERT_TRUE(result.has_value()); - ASSERT_TRUE(result.value().has_value()); - EXPECT_EQ(result.value().value(), expected_hash); -} - -TEST_F(CompletionSlotTest, PreCancelledStopTokenAbortsWait) -{ - RecordProperty("Description", "Verify CompletionSlot::wait returns nullopt if stop_token is already stopped."); - - CompletionSlot slot; - score::cpp::stop_source source; - source.request_stop(); - - auto result = slot.wait(source.get_token()); - EXPECT_FALSE(result.has_value()); + EXPECT_EQ(result.value(), expected_hash); } TEST_F(CompletionSlotTest, ZeroAllocationInCompleteAndWait) @@ -141,8 +126,7 @@ TEST_F(CompletionSlotTest, ZeroAllocationInCompleteAndWait) slot.complete(score::Result{test_hash}); auto result = slot.wait(); ASSERT_TRUE(result.has_value()); - ASSERT_TRUE(result.value().has_value()); - EXPECT_EQ(result.value().value(), test_hash); + EXPECT_EQ(result.value(), test_hash); } g_tracking_enabled = false; @@ -160,8 +144,7 @@ TEST_F(CompletionSlotTest, ZeroAllocationInResultVoidCompleteAndWait) CompletionSlot> slot; slot.complete(score::Result{}); auto result = slot.wait(); - ASSERT_TRUE(result.has_value()); - EXPECT_TRUE(result.value().has_value()); + EXPECT_TRUE(result.has_value()); } g_tracking_enabled = false; 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 bad7575838..b758bd00dc 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 @@ -461,9 +461,7 @@ Result ProcessGroupManager::getActiveRunTarget() const noexcept return score::MakeUnexpected(ExecErrc::kFailed); } - const auto get_result = slot.wait(cpp::stop_token{}); - SCORE_LANGUAGE_FUTURECPP_ASSERT(get_result.has_value()); - return get_result.value(); + return slot.wait(); } void ProcessGroupManager::handleSetRequestedRunTarget(SetRequestedRunTarget* event) noexcept @@ -518,9 +516,7 @@ Result ProcessGroupManager::setRequestedRunTarget(IdentifierHash run_targe return score::MakeUnexpected(ExecErrc::kFailed); } - const auto get_result = slot.wait(cpp::stop_token{}); - SCORE_LANGUAGE_FUTURECPP_ASSERT(get_result.has_value()); - return get_result.value(); + return slot.wait(); } void ProcessGroupManager::registerActiveRunTargetCallback(ActivationCallbackT callback) noexcept From 9719e29d1a1482832e075c46bd8ce3cc0eeef848 Mon Sep 17 00:00:00 2001 From: mhossamr Date: Mon, 19 Oct 2026 13:08:15 +0300 Subject: [PATCH 3/3] style(launch_manager): format unit tests --- .../process_group_manager/details/completion_slot_UT.cpp | 3 ++- .../details/component_event_queue_UT.cpp | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/completion_slot_UT.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/completion_slot_UT.cpp index 891b13fc4e..1b6625fcd1 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/completion_slot_UT.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/completion_slot_UT.cpp @@ -114,7 +114,8 @@ TEST_F(CompletionSlotTest, AsynchronousCompleteFromWorkerThread) TEST_F(CompletionSlotTest, ZeroAllocationInCompleteAndWait) { - RecordProperty("Description", "Verify CompletionSlot complete() and wait() perform zero dynamic memory allocations."); + RecordProperty( + "Description", "Verify CompletionSlot complete() and wait() perform zero dynamic memory allocations."); const IdentifierHash test_hash{"Running"}; diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/component_event_queue_UT.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/component_event_queue_UT.cpp index 3a8eb3b8f3..67d5e0c133 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/component_event_queue_UT.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/component_event_queue_UT.cpp @@ -83,8 +83,7 @@ TEST_F(ComponentEventQueueTest, GetNextEventReturnsSupervisionFailureWithPayload TEST_F(ComponentEventQueueTest, GetNextEventReturnsGetActiveRunTargetWithCompletionSlot) { RecordProperty( - "Description", - "Verify a pushed GetActiveRunTarget event is returned with its CompletionSlot pointer intact."); + "Description", "Verify a pushed GetActiveRunTarget event is returned with its CompletionSlot pointer intact."); CompletionSlot> slot; EXPECT_TRUE(queue_.push(GetActiveRunTarget{completion : &slot})); @@ -99,7 +98,8 @@ TEST_F(ComponentEventQueueTest, GetNextEventReturnsSetRequestedRunTargetWithPayl { RecordProperty( "Description", - "Verify a pushed SetRequestedRunTarget event is returned with its run_target and CompletionSlot pointer intact."); + "Verify a pushed SetRequestedRunTarget event is returned with its run_target and CompletionSlot pointer " + "intact."); CompletionSlot> slot; const IdentifierHash target{"RunTargetA"}; EXPECT_TRUE(queue_.push(SetRequestedRunTarget{run_target : target, completion : &slot}));