From 8dfa465f310ace3a6990371877ead238440efdb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicolas=20Fu=C3=9Fberger?= Date: Wed, 14 Oct 2026 11:08:40 +0200 Subject: [PATCH] Small oshandler improvement --- .../details/os_handler.cpp | 38 +++++++++++++------ .../details/oshandler_UT.cpp | 31 +++++++++++++++ .../details/process_info_node.cpp | 18 ++++----- .../details/process_info_node.hpp | 4 +- 4 files changed, 69 insertions(+), 22 deletions(-) diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/os_handler.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/os_handler.cpp index 3430dc1ec8..01e3fa878d 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/os_handler.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/os_handler.cpp @@ -13,6 +13,8 @@ #include "score/mw/launch_manager/process_group_manager/details/os_handler.hpp" +#include + namespace score::mw::lifecycle::internal { @@ -21,23 +23,37 @@ void OsHandler::run(void) while (is_running_) { int32_t wait_status = 0; - auto result = sys_wait_.wait(&wait_status); + const auto result = sys_wait_.wait(&wait_status); - if (result.has_value() && result.value() > 0) - { - if (score::mw::lifecycle::internal::SafeProcessMapReturnType::kInsertionError == - 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 + if (!result.has_value() || (result.value() <= 0)) { // This process has no children to wait for at present, // or the wait was interrupted by a signal. // Wait a while to stop this thread from hogging cpu time std::this_thread::sleep_for(OS_HANDLER_LOOP_DELAY); + continue; + } + + if (WIFSTOPPED(wait_status) || WIFCONTINUED(wait_status)) + { + // Child was stopped or continues, not terminated. Nothing to do. + continue; + } + + if (WIFEXITED(wait_status)) + { + LM_LOG_DEBUG() << "PID" << result.value() << "exited with exit code" << WEXITSTATUS(wait_status); + } + else if (WIFSIGNALED(wait_status)) + { + LM_LOG_DEBUG() << "PID" << result.value() << "was killed by signal" << WTERMSIG(wait_status); + } + + if (score::mw::lifecycle::internal::SafeProcessMapReturnType::kInsertionError == + 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)."; } } } diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/oshandler_UT.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/oshandler_UT.cpp index 1f3b8febb4..abd16b51e3 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/oshandler_UT.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/oshandler_UT.cpp @@ -215,4 +215,35 @@ TEST_F(OsHandlerTest, WaitReturnsErrorThenProcessId_HandlerRecoversAndInvokesCal sut_.reset(); } +TEST_F(OsHandlerTest, WaitReturnsStoppedOrContinuedStatus_FindTerminatedIsNotCalled) +{ + RecordProperty( + "Description", + "When sys_wait reports a stopped or continued child, OsHandler must not treat the process as terminated."); + + // given + StrictMock callback; + process_map_.insertIfNotTerminated(6000, &callback); + + constexpr int32_t kStoppedStatus = 0x7f | (SIGSTOP << 8); + constexpr int32_t kContinuedStatus = 0xffff; + EXPECT_CALL(*sys_wait_mock_, wait(_)) + .WillOnce([](std::int32_t* stat_loc) -> score::cpp::expected { + *stat_loc = kStoppedStatus; + return 6000; + }) + .WillOnce([](std::int32_t* stat_loc) -> score::cpp::expected { + *stat_loc = kContinuedStatus; + return 6000; + }) + .WillRepeatedly(Return(score::cpp::unexpected(score::os::Error::createFromErrno(ECHILD)))); + + // when + sut_ = std::make_unique(process_map_, *sys_wait_mock_); + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + sut_.reset(); + + // then — StrictMock ensures terminated() was never called +} + } // namespace 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 cc5253234d..2a56adbd45 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 @@ -27,7 +27,7 @@ namespace score::mw::lifecycle::internal ProcessInfoNode::ProcessInfoNode(configuration::ComponentConfig&& config, ProcessHandling process_handling) : termination_requested_(false), pid_(0), - exit_code_(0), + exit_status_(0), config_(std::move(config)), process_handling_(std::move(process_handling)), identifier_(config_.name), @@ -174,8 +174,8 @@ void ProcessInfoNode::unblockSync() IComponent::RequestResult ProcessInfoNode::tryHandleTermination(int32_t process_status) { - LM_LOG_DEBUG() << "Process" << identifier_ << "( pid" << pid_ << ") terminated with exit code" << process_status; - exit_code_ = process_status; + LM_LOG_DEBUG() << "Process" << identifier_ << "( pid" << pid_ << ") terminated with status" << process_status; + exit_status_ = process_status; IComponent::RequestResult res = {IComponent::RequestState::kWaiting}; ProcessState starting = ProcessState::kStarting; @@ -211,8 +211,8 @@ IComponent::RequestResult ProcessInfoNode::tryHandleTermination(int32_t process_ } else { - LM_LOG_WARN() << "unexpected termination of process" << identifier_ << "( pid" << pid_ << "exit code" - << exit_code_ << ")"; + LM_LOG_WARN() << "unexpected termination of process" << identifier_ << "( pid" << pid_ << "status" + << exit_status_ << ")"; res = score::cpp::make_unexpected(IComponent::ComponentError::kErrorAfterReady); } } @@ -259,7 +259,7 @@ IComponent::RequestResult ProcessInfoNode::startProcess(score::cpp::stop_token s } pid_ = 0; - exit_code_ = 0; + exit_status_ = 0; error = std::nullopt; termination_result_ = TeminationResult::kNone; termination_waiter_->reset(); @@ -366,7 +366,7 @@ score::cpp::expected_blank ProcessInfoNode::handlePr // we can return early if (!isReporting() && (*ready_state == configuration::ProcessState::Running)) { - return exit_code_ == 0; + return exit_status_ == 0; } // We only want to wait for kRunning with Reporting processes @@ -394,7 +394,7 @@ score::cpp::expected_blank ProcessInfoNode::handlePr } } - return exit_code_ == 0; + return exit_status_ == 0; } // req-id: comp_req__launch_man__rc_file_state else if constexpr (std::is_same_v) @@ -420,7 +420,7 @@ score::cpp::expected_blank ProcessInfoNode::handlePr LM_LOG_ERROR() << "Error Waiting for file"; } - return (wait_res == osal::OsalReturnType::kSuccess) && (exit_code_ == 0); + return (wait_res == osal::OsalReturnType::kSuccess) && (exit_status_ == 0); } }, config_.component_properties.ready_condition); 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 0589a52e89..9c7561e2ff 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 @@ -61,7 +61,7 @@ class ProcessInfoNode final : public IComponent ProcessInfoNode(ProcessInfoNode&& other) noexcept : termination_requested_(other.termination_requested_.load()), pid_(other.pid_), - exit_code_(other.exit_code_.load()), + exit_status_(other.exit_status_.load()), process_state_(other.process_state_.load()), reached_ready_(other.reached_ready_.load()), config_(std::move(other.config_)), @@ -172,7 +172,7 @@ class ProcessInfoNode final : public IComponent osal::ProcessID pid_ = 0; /// @brief The status reported by the operating system when the process terminated - std::atomic exit_code_{0}; + std::atomic exit_status_{0}; /// @brief The current state of the OS process std::atomic process_state_{score::mw::lifecycle::ProcessState::kIdle};