Add Unit tests for SafeProcessMap and OSHandler class

score/launch_manager/daemon/src/process_group_manager/details/BUILD

@@ -19,6 +19,7 @@ cc_library(
     strip_include_prefix = "/score/launch_manager/daemon/src/process_group_manager/details",
     visibility = ["//score/launch_manager/daemon/src/process_group_manager:__pkg__"],
     deps = [
+        ":safe_process_map",
         "//score/launch_manager/daemon/src/configuration:configuration_manager",
         "//score/launch_manager/daemon/src/control:control_client_channel",
         "//score/launch_manager/daemon/src/osal:ipc_comms",
@@ -35,6 +36,7 @@ cc_library(
     visibility = ["//score/launch_manager/daemon/src/process_group_manager:__pkg__"],
     deps = [
         ":process_info_node",
+        ":safe_process_map",
         "//score/launch_manager/daemon/src/common:identifier_hash",
         "//score/launch_manager/daemon/src/configuration:configuration_manager",
         "//score/launch_manager/daemon/src/control:control_client_channel",
@@ -51,11 +53,19 @@ cc_library(
     strip_include_prefix = "/score/launch_manager/daemon/src/process_group_manager/details",
     visibility = ["//score/launch_manager/daemon/src/process_group_manager:__pkg__"],
     deps = [
-        ":process_info_node",
         "//score/launch_manager/daemon/src/process_group_manager:iprocess",
     ],
 )
 
+cc_test(
+    name = "safeprocessmap_UT",
+    srcs = ["safeprocessmap_UT.cpp"],
+    deps = [
+        ":safe_process_map",
+        "@googletest//:gtest_main",
+    ],
+)
+
 cc_library(
     name = "os_handler",
     srcs = ["os_handler.cpp"],
@@ -67,6 +77,18 @@ cc_library(
         ":safe_process_map",
         "//score/launch_manager/daemon/src/common:log",
         "//score/launch_manager/daemon/src/process_group_manager:iprocess",
+        "@score_baselibs//score/os:sys_wait",
+    ],
+)
+
+cc_test(
+    name = "oshandler_UT",
+    srcs = ["oshandler_UT.cpp"],
+    deps = [
+        ":os_handler",
+        ":safe_process_map",
+        "@googletest//:gtest_main",
+        "@score_baselibs//score/os/mocklib:sys_wait_mock",
     ],
 )
 

score/launch_manager/daemon/src/process_group_manager/details/os_handler.cpp

@@ -21,12 +21,12 @@ namespace internal {
 
 void OsHandler::run(void) {
     while (is_running_) {
-        int32_t status = 0;
-        osal::ProcessID targetProcessId = 0;
+        int32_t wait_status = 0;
+        auto result = sys_wait_.wait(&wait_status);
 
-        if (osal::OsalReturnType::kSuccess == process_interface_.waitForTermination(targetProcessId, status)) {
-            if (-1 == safe_process_map_.findTerminated(targetProcessId, status)) {
-                LM_LOG_ERROR() << "[os handler: out of resources]";
+        if (result.has_value() && result.value() > 0) {
+            if (-1 == safe_process_map_.findTerminated(result.value(), wait_status)) {
+                LM_LOG_ERROR() << "No more resources available to track process with PID " << result.value() << "(SafeProcessMap capacity exceeded).";
             }
         } else {
             // This process has no children to wait for at present,

score/launch_manager/daemon/src/process_group_manager/details/os_handler.hpp

@@ -18,7 +18,7 @@
 #include <chrono>
 #include <thread>
 
-#include "score/mw/launch_manager/process_group_manager/iprocess.hpp"
+#include "score/os/sys_wait.h"
 #include "score/mw/launch_manager/process_group_manager/details/safe_process_map.hpp"
 
 namespace score {
@@ -39,15 +39,13 @@ constexpr std::chrono::milliseconds OS_HANDLER_LOOP_DELAY{100};  // TODO - Defin
 /// There will only be one instance of OsHandler during the Launch Manager's lifetime.
 class OsHandler final {
    public:
-    /// @brief Constructs an OsHandler with safe process map and OS abstraction layer process interfaces
+    /// @brief Constructs an OsHandler with safe process map and SysWait interface.
     /// This constructor initializes the OsHandler, starts its execution thread, and prepares it to handle process terminations.
     /// @param map A reference to a SafeProcessMap that stores the mapping of processes to be managed.
-    /// @param process_interface A reference to an implementation of osal::IProcess, which provides the necessary
-    /// methods for process management, including waiting for process termination.
-    /// @note The lifetime of the object passed as `process_interface` must extend at least as long as
-    /// the lifetime of this OsHandler instance to avoid accessing dangling references.
-    OsHandler(SafeProcessMap& map, osal::IProcess& process_interface)
-        : safe_process_map_(map), process_interface_(process_interface) {
+    /// @param sys_wait A reference to a score::os::SysWait instance used to wait for child process termination.
+    ///        Defaults to the production singleton. A mock can be injected for testing.
+    OsHandler(SafeProcessMap& map, score::os::SysWait& sys_wait = score::os::SysWait::instance())
+        : safe_process_map_(map), sys_wait_(sys_wait) {
     }
 
     /// @brief Stops and and destroy the execution of the OsHandler's thread by setting the is_running_ flag to false,
@@ -84,8 +82,8 @@ class OsHandler final {
     /// @brief Indicates whether the os handler's thread is currently running.
     std::atomic_bool is_running_{true};
 
-    /// @brief Interface to the process functionality provided by the OSAL.
-    osal::IProcess& process_interface_;
+    /// @brief Interface to wait for child process termination.
+    score::os::SysWait& sys_wait_;
 
     /// @brief Thread object to manage execution of the run method.
     std::thread os_handler_{&score::lcm::internal::OsHandler::run, this};

score/launch_manager/daemon/src/process_group_manager/details/oshandler_UT.cpp

@@ -0,0 +1,218 @@
+/********************************************************************************
+ * Copyright (c) 2025 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 <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+#include <chrono>
+#include <memory>
+#include <thread>
+
+#include "score/mw/launch_manager/process_group_manager/details/os_handler.hpp"
+
+#include "score/mw/launch_manager/process_group_manager/details/safe_process_map.hpp"
+#include "score/mw/launch_manager/common/constants.hpp"
+#include "score/os/mocklib/sys_wait_mock.h"
+
+using namespace testing;
+using namespace score::lcm::internal;
+
+namespace
+{
+
+class MockTerminationCallback : public ITerminationCallback
+{
+   public:
+    MOCK_METHOD(void, terminated, (int32_t process_status), (override));
+};
+
+class OsHandlerTest : public ::testing::Test
+{
+   protected:
+    void SetUp() override
+    {
+        RecordProperty("TestType", "interface-test");
+        RecordProperty("DerivationTechnique", "explorative-testing");
+    }
+
+    static constexpr uint32_t kCapacity = 32U;
+
+    SafeProcessMap process_map_{kCapacity};
+    score::os::MockGuard<score::os::SysWaitMock> sys_wait_mock_;
+    std::unique_ptr<OsHandler> sut_;
+};
+
+TEST_F(OsHandlerTest, WaitReturnsProcessId_FindTerminatedIsCalled)
+{
+    RecordProperty("Description",
+                   "When sys_wait returns a valid pid, OsHandler calls findTerminated and the termination callback "
+                   "is invoked.");
+
+    // given — insert a callback for pid 1000
+    NiceMock<MockTerminationCallback> callback;
+    process_map_.insertIfNotTerminated(1000, &callback);
+
+    constexpr int32_t kExitStatus = 42;
+    EXPECT_CALL(callback, terminated(kExitStatus)).Times(AtLeast(1));
+
+    // sys_wait returns pid 1000 once, then blocks with error
+    EXPECT_CALL(*sys_wait_mock_, wait(_))
+        .WillOnce([](std::int32_t* stat_loc) -> score::cpp::expected<pid_t, score::os::Error> {
+            *stat_loc = kExitStatus;
+            return 1000;
+        })
+        .WillRepeatedly(Return(score::cpp::unexpected(score::os::Error::createFromErrno(ECHILD))));
+
+    // when
+    sut_ = std::make_unique<OsHandler>(process_map_, *sys_wait_mock_);
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+    sut_.reset();
+}
+
+TEST_F(OsHandlerTest, WaitReturnsError_OsHandlerSleepsAndDoesNotCallFindTerminated)
+{
+    RecordProperty("Description",
+                   "When sys_wait returns an error, OsHandler sleeps and does not call findTerminated.");
+
+    // given — insert a callback that should NOT be invoked
+    StrictMock<MockTerminationCallback> callback;
+    process_map_.insertIfNotTerminated(2000, &callback);
+
+    EXPECT_CALL(*sys_wait_mock_, wait(_))
+        .WillRepeatedly(Return(score::cpp::unexpected(score::os::Error::createFromErrno(ECHILD))));
+
+    // when
+    sut_ = std::make_unique<OsHandler>(process_map_, *sys_wait_mock_);
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+    sut_.reset();
+
+    // then — StrictMock ensures terminated() was never called
+}
+
+TEST_F(OsHandlerTest, WaitReturnsZeroPid_OsHandlerSleepsAndDoesNotCallFindTerminated)
+{
+    RecordProperty("Description",
+                   "When sys_wait returns pid 0, OsHandler sleeps and does not call findTerminated.");
+
+    // given
+    StrictMock<MockTerminationCallback> callback;
+    process_map_.insertIfNotTerminated(3000, &callback);
+
+    EXPECT_CALL(*sys_wait_mock_, wait(_)).WillRepeatedly(Return(score::cpp::expected<pid_t, score::os::Error>{0}));
+
+    // when
+    sut_ = std::make_unique<OsHandler>(process_map_, *sys_wait_mock_);
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+    sut_.reset();
+
+    // then — StrictMock ensures terminated() was never called
+}
+
+TEST_F(OsHandlerTest, WaitReturnsProcessIdBeforeRegistration_LaterRegistrationReceivesStoredTermination)
+{
+    RecordProperty("Description",
+                   "Covers the race where a child terminates before the process has been registered in "
+                   "SafeProcessMap. OsHandler sees the pid first, stores the terminated state, and a later "
+                   "insertIfNotTerminated call must immediately deliver the stored exit status to the callback.");
+
+    // given
+    // Simulate the OS reporting that pid 4000 has already exited.
+    // At this point nothing has been inserted into SafeProcessMap yet.
+    std::atomic_bool first_wait_seen{false};
+
+    EXPECT_CALL(*sys_wait_mock_, wait(_))
+        .WillOnce([&first_wait_seen](std::int32_t* stat_loc) -> score::cpp::expected<pid_t, score::os::Error> {
+            *stat_loc = 99;
+            first_wait_seen.store(true);
+            return 4000;
+        })
+        .WillRepeatedly(Return(score::cpp::unexpected(score::os::Error::createFromErrno(ECHILD))));
+
+    // when
+    // Start OsHandler and wait until its background thread has observed that terminated pid.
+    // That should cause OsHandler to call SafeProcessMap::findTerminated(4000, 99), which stores
+    // "pid 4000 already terminated with status 99" because no callback is registered yet.
+    sut_ = std::make_unique<OsHandler>(process_map_, *sys_wait_mock_);
+    while (!first_wait_seen.load())
+    {
+        std::this_thread::yield();
+    }
+    // Allow OsHandler thread to complete findTerminated() after wait() returned.
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+    // then
+    // Register the process after the termination has already been observed.
+    // insertIfNotTerminated must detect the previously stored termination, return 1, and invoke the
+    // callback immediately with the saved exit status instead of creating a new live entry.
+    StrictMock<MockTerminationCallback> callback;
+    EXPECT_CALL(callback, terminated(99)).Times(1);
+    EXPECT_EQ(process_map_.insertIfNotTerminated(4000, &callback), 1);
+
+    sut_.reset();
+}
+
+TEST_F(OsHandlerTest, WaitReturnsUnknownPidWhenMapIsFull_OutOfResourcesPathDoesNotNotifyCallbacks)
+{
+    RecordProperty("Description",
+                   "When sys_wait reports an unknown pid and the map is full, OsHandler takes the out-of-resources "
+                   "path without notifying tracked callbacks.");
+
+    // given
+    StrictMock<MockTerminationCallback> callbacks[kCapacity];
+    for (uint32_t i = 0; i < kCapacity; ++i)
+    {
+        ASSERT_EQ(process_map_.insertIfNotTerminated(static_cast<int32_t>(i + 1U), &callbacks[i]), 0);
+    }
+
+    EXPECT_CALL(*sys_wait_mock_, wait(_))
+        .WillOnce([](std::int32_t* stat_loc) -> score::cpp::expected<pid_t, score::os::Error> {
+            *stat_loc = 17;
+            return 9999;
+        })
+        .WillRepeatedly(Return(score::cpp::unexpected(score::os::Error::createFromErrno(ECHILD))));
+
+    // when
+    sut_ = std::make_unique<OsHandler>(process_map_, *sys_wait_mock_);
+    std::this_thread::sleep_for(std::chrono::milliseconds(50));
+    sut_.reset();
+
+    // then — StrictMock ensures no tracked callback was invoked
+}
+
+TEST_F(OsHandlerTest, WaitReturnsErrorThenProcessId_HandlerRecoversAndInvokesCallback)
+{
+    RecordProperty("Description",
+                   "When sys_wait first returns an error and then a valid pid, OsHandler resumes processing and "
+                   "invokes the callback.");
+
+    // given
+    NiceMock<MockTerminationCallback> callback;
+    process_map_.insertIfNotTerminated(5000, &callback);
+
+    EXPECT_CALL(callback, terminated(7)).Times(AtLeast(1));
+
+    EXPECT_CALL(*sys_wait_mock_, wait(_))
+        .WillOnce(Return(score::cpp::unexpected(score::os::Error::createFromErrno(ECHILD))))
+        .WillOnce([](std::int32_t* stat_loc) -> score::cpp::expected<pid_t, score::os::Error> {
+            *stat_loc = 7;
+            return 5000;
+        })
+        .WillRepeatedly(Return(score::cpp::unexpected(score::os::Error::createFromErrno(ECHILD))));
+
+    // when
+    sut_ = std::make_unique<OsHandler>(process_map_, *sys_wait_mock_);
+    std::this_thread::sleep_for(OS_HANDLER_LOOP_DELAY + std::chrono::milliseconds(50));
+    sut_.reset();
+}
+
+}  // namespace

score/launch_manager/daemon/src/process_group_manager/details/process_group_manager.cpp

@@ -264,7 +264,7 @@ bool ProcessGroupManager::run()
                    // debug messages.
                    << (static_cast<double>(clock()) / (static_cast<double>(CLOCKS_PER_SEC) / 1000.0)) << "ms";
 
-    OsHandler os_handler(*process_map_, process_interface_);
+    OsHandler os_handler(*process_map_);
 
     bool result = startInitialTransition();
 

score/launch_manager/daemon/src/process_group_manager/details/process_info_node.hpp

@@ -16,6 +16,7 @@
 
 #include <atomic>
 #include "score/mw/launch_manager/configuration/configuration_manager.hpp"
+#include "score/mw/launch_manager/process_group_manager/details/safe_process_map.hpp"
 #include "score/mw/launch_manager/control/control_client_channel.hpp"
 
 namespace score {
@@ -41,7 +42,7 @@ using SuccessorList = std::vector<std::shared_ptr<ProcessInfoNode>>;
 /// the events kRunning received or expected process termination forming connecting edges.
 /// Unexpected termination of a process or reception of a timeout result in the failure of this graph.
 /// The ProcessInfoNode thus represents an adaptive process with specific startup configuration and dependencies
-class ProcessInfoNode final {
+class ProcessInfoNode final : public ITerminationCallback {
    public:
     /// Constructor for Process Infonode
     ProcessInfoNode()
@@ -77,7 +78,7 @@ class ProcessInfoNode final {
     /// The process status is saved in the ProcessInfoNode
     /// This method will be called by the OsHander.
     /// @param process_status - the value returned by the OS for the process termination status
-    void terminated(int32_t process_status);
+    void terminated(int32_t process_status) override;
 
     /// @brief Called by a worker thread to perform the expected action for this node
     /// The action will either be to start the process, if graph_->isStarting() returns true,

score/launch_manager/daemon/src/process_group_manager/details/safe_process_map.cpp

@@ -14,8 +14,8 @@
 #include <cstdio>
 #include <cstdlib>
 #include <thread>
+
 #include "score/mw/launch_manager/process_group_manager/details/safe_process_map.hpp"
-#include "score/mw/launch_manager/process_group_manager/details/process_info_node.hpp"
 
 namespace score {
 
@@ -99,7 +99,7 @@ inline int32_t SafeProcessMap::removeNode(ProcessInfoData& target, ProcessInfoDa
     // found key. There are 4 situations:
     // data.pin_ == nullptr, stored pin_ != nullptr: normal findTerminated
     // data.pin_ != nullptr, stored pin_ == nullptr: normal insertIfNotTerminated
-    // both data.pin_ and stored pin_ point to a ProcessInfoNode: anomalous
+    // both data.pin_ and stored pin_ point to an ITerminationCallback: anomalous
     // both data.pin_ and stored pin_ are null: anomalous
     // In other words, exactly one of data.pin_ and stored pin_ must be nullptr
     // or there is an anomaly and we return -2
@@ -239,7 +239,7 @@ int32_t SafeProcessMap::findTerminated(osal::ProcessID key, int32_t status) {
     return search(key, {status, nullptr});
 }
 
-int32_t SafeProcessMap::insertIfNotTerminated(osal::ProcessID key, ProcessInfoNode* object) {
+int32_t SafeProcessMap::insertIfNotTerminated(osal::ProcessID key, ITerminationCallback* object) {
     return search(key, {0, object});
 }