From dafee9b8bb733dc51bcfd4472dc93b9f45b70604 Mon Sep 17 00:00:00 2001 From: Maciej Kaszynski Date: Tue, 21 Jul 2026 08:21:13 +0100 Subject: [PATCH 1/7] Adding notify headers --- .../launch_manager/src/daemon/src/osal/BUILD | 34 ++- .../osal/details/posix/inotify_iterator.cpp | 202 ++++++++++++++++++ .../details/posix/inotify_iterator_UT.cpp | 87 ++++++++ .../src/daemon/src/osal/inotify_iterator.hpp | 139 ++++++++++++ .../src/process_group_manager/details/BUILD | 12 +- 5 files changed, 467 insertions(+), 7 deletions(-) create mode 100644 score/launch_manager/src/daemon/src/osal/details/posix/inotify_iterator.cpp create mode 100644 score/launch_manager/src/daemon/src/osal/details/posix/inotify_iterator_UT.cpp create mode 100644 score/launch_manager/src/daemon/src/osal/inotify_iterator.hpp diff --git a/score/launch_manager/src/daemon/src/osal/BUILD b/score/launch_manager/src/daemon/src/osal/BUILD index 8c4b45587..e64446e3f 100644 --- a/score/launch_manager/src/daemon/src/osal/BUILD +++ b/score/launch_manager/src/daemon/src/osal/BUILD @@ -10,7 +10,7 @@ # # SPDX-License-Identifier: Apache-2.0 # ******************************************************************************* -load("@rules_cc//cc:defs.bzl", "cc_library") +load("@rules_cc//cc:defs.bzl", "cc_library", "cc_test") cc_library( name = "return_types", @@ -109,6 +109,38 @@ cc_library( ], ) + +cc_library( + name = "inotify_iterator", + hdrs = [ + "inotify_iterator.hpp", + ], + srcs = [ + "details/posix/inotify_iterator.cpp" + ], + include_prefix = "score/mw/launch_manager/osal", + strip_include_prefix = "/score/launch_manager/src/daemon/src/osal", + visibility = [ + "//score:__subpackages__", + "//tests:__subpackages__", + ], + deps = [ + "//score/launch_manager/src/daemon/src/common:log", + "@score_baselibs//score/result", + "//score/launch_manager:error" + ], +) + +cc_test( + name = "inotify_iterator_UT", + srcs = ["details/posix/inotify_iterator_UT.cpp"], + visibility = ["//tests:__subpackages__"], + deps = [ + ":inotify_iterator", + "@googletest//:gtest_main", + ], +) + cc_library( name = "osal", visibility = ["//score:__subpackages__"], diff --git a/score/launch_manager/src/daemon/src/osal/details/posix/inotify_iterator.cpp b/score/launch_manager/src/daemon/src/osal/details/posix/inotify_iterator.cpp new file mode 100644 index 000000000..19e9c62ba --- /dev/null +++ b/score/launch_manager/src/daemon/src/osal/details/posix/inotify_iterator.cpp @@ -0,0 +1,202 @@ +#include "score/mw/launch_manager/osal/inotify_iterator.hpp" +#include "score/launch_manager/src/daemon/src/common/log.hpp" +#include "score/launch_manager/src/execution_error.h" +#include +#include + +#include +#include + +INotifyWatcher::INotifyWatcher(int event_queue_fd, int event_fd, int epoll_fd) + : event_queue_fd_(event_queue_fd), event_fd_(event_fd), epoll_fd_(epoll_fd) +{ + + auto add = [&](int fd) { + epoll_event ev{}; + ev.events = EPOLLIN; + ev.data.fd = fd; + ::epoll_ctl(epoll_fd_, EPOLL_CTL_ADD, fd, &ev); + }; + add(event_queue_fd_); + add(event_fd_); +} + +score::Result INotifyWatcher::Create() noexcept +{ + int event_queue_fd = ::inotify_init1(IN_CLOEXEC); + int event_fd = ::eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK); + int epoll_fd = ::epoll_create1(EPOLL_CLOEXEC); + + if (event_queue_fd < 0 || event_fd < 0 || epoll_fd < 0) + { + score::Result{score::MakeUnexpected(score::mw::lifecycle::ExecErrc::kCommunicationError)}; + } + + return INotifyWatcher{event_queue_fd, event_fd, epoll_fd}; +} + +INotifyWatcher::INotifyWatcher(INotifyWatcher&& other) noexcept + : event_queue_fd_(other.event_queue_fd_), event_fd_(other.event_fd_), epoll_fd_(other.epoll_fd_) +{ + // invalidate others file descriptors + other.event_queue_fd_ = -1; + other.event_fd_ = -1; + other.epoll_fd_ = -1; +} + +INotifyWatcher& INotifyWatcher::operator=(INotifyWatcher&& other) noexcept +{ + event_queue_fd_ = other.event_queue_fd_; + event_fd_ = other.event_fd_; + epoll_fd_ = other.epoll_fd_; + + // invalidate others file descriptors + other.event_queue_fd_ = -1; + other.event_fd_ = -1; + other.epoll_fd_ = -1; + return *this; +} + +INotifyWatcher::~INotifyWatcher() +{ + static_cast(::close(epoll_fd_)); + static_cast(::close(event_fd_)); + static_cast(::close(event_queue_fd_)); +} + +int INotifyWatcher::add_watch(const std::string_view path, uint32_t mask) const noexcept +{ + return ::inotify_add_watch(event_queue_fd_, path.begin(), mask); +} + +void INotifyWatcher::interrupt() const +{ + std::uint64_t val = 1; + ::write(event_fd_, &val, sizeof(val)); +} + +INotifyWatcher::iterator INotifyWatcher::begin() +{ + SCORE_LANGUAGE_FUTURECPP_ASSERT_MESSAGE(event_fd_ > 0, "Event FD not initlized!"); + SCORE_LANGUAGE_FUTURECPP_ASSERT_MESSAGE(epoll_fd_ > 0, "Event FD not initlized!"); + SCORE_LANGUAGE_FUTURECPP_ASSERT_MESSAGE(event_queue_fd_ > 0, "Event FD not initlized!"); + + iterator out{this}; + ++out; + return out; +} + +INotifyWatcher::iterator INotifyWatcher::end() +{ + return iterator{nullptr}; +} + +INotifyWatcher::iterator::iterator(INotifyWatcher* watch_ptr) : watcher_(watch_ptr) {}; + +INotifyWatcher::iterator::reference INotifyWatcher::iterator::operator*() const +{ + return current_; +} + +INotifyWatcher::iterator::pointer INotifyWatcher::iterator::operator->() const +{ + return ¤t_; +} + +INotifyWatcher::iterator& INotifyWatcher::iterator::operator++() +{ + if (watcher_ == nullptr) + { + // don't advance over end() + return *this; + } + if (!advance()) + { + // become end() + watcher_ = nullptr; + } + return *this; +} + +INotifyWatcher::iterator INotifyWatcher::iterator::operator++(int) +{ + auto tmp = *this; + ++(*this); + return tmp; +} + +bool INotifyWatcher::iterator::operator==(const iterator& other) const +{ + return watcher_ == other.watcher_; +} + +bool INotifyWatcher::iterator::operator!=(const iterator& other) const +{ + return !(*this == other); +} + +bool INotifyWatcher::iterator::advance() +{ + const bool events_in_buffer = buf_pos_ < buf_len_; + if (events_in_buffer) + { + consume_next(); + return true; + } + + // need 2 fields, one for the inotify fd and one for the event fd + std::array events{}; + int events_seen = ::epoll_wait(watcher_->epoll_fd_, events.data(), events.size(), -1); + + if (events_seen < 0) + { + LM_LOG_ERROR() << "Failed to epoll_wait" << std::strerror(errno); + return false; + } + + for (const auto& event : events) + { + if (event.data.fd == watcher_->event_fd_) + { + // recived event from event_fd_ so we got interrupted + std::uint64_t val = 0; + ::read(watcher_->event_fd_, &val, sizeof(val)); + return false; + } + } + + ssize_t bytes_read = ::read(watcher_->event_queue_fd_, buf_.data(), buf_.size()); + if (bytes_read <= 0) + { + // this shouldn't happen but we should still continue... + return false; + } + + buf_len_ = static_cast(bytes_read); + buf_pos_ = 0; + + consume_next(); + return true; +}; + +void INotifyWatcher::iterator::consume_next() +{ + const char* raw = std::next(buf_.data(), static_cast(buf_pos_)); + + // using memcpy so that we don't reintepret_cast + std::memcpy(¤t_.wd, raw, sizeof(inotify_event::wd)); + raw = std::next(raw, sizeof(inotify_event::wd)); + + std::memcpy(¤t_.mask, raw, sizeof(inotify_event::mask)); + raw = std::next(raw, sizeof(inotify_event::mask)); + + std::memcpy(¤t_.cookie, raw, sizeof(inotify_event::cookie)); + raw = std::next(raw, sizeof(inotify_event::cookie)); + + decltype(inotify_event::len) len {0}; + std::memcpy(&len, raw, sizeof(inotify_event::len)); + raw = std::next(raw, sizeof(inotify_event::len)); + + current_.name = std::string_view(raw, len); + buf_pos_ += static_cast(sizeof(inotify_event)) + len; +}; diff --git a/score/launch_manager/src/daemon/src/osal/details/posix/inotify_iterator_UT.cpp b/score/launch_manager/src/daemon/src/osal/details/posix/inotify_iterator_UT.cpp new file mode 100644 index 000000000..a9eaae732 --- /dev/null +++ b/score/launch_manager/src/daemon/src/osal/details/posix/inotify_iterator_UT.cpp @@ -0,0 +1,87 @@ +/******************************************************************************** + * 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 "gtest/gtest.h" +#include "score/mw/launch_manager/osal/inotify_iterator.hpp" +#include +#include +#include +#include +#include + +class INotifyTest : public ::testing::Test +{ + protected: + + constexpr static std::string_view TEST_FILE = "/tmp/test"; + constexpr static std::string_view TEST_DIR = "/tmp"; + + void SetUp() override + { + RecordProperty("TestType", "interface-test"); + RecordProperty("DerivationTechnique", "explorative-testing "); + } + + void TearDown() override + { + ::unlink(TEST_FILE.data()); + } + + static ::testing::AssertionResult touch_file(std::string_view path = TEST_FILE) + { + auto fd = ::open(path.begin(), O_CREAT | O_WRONLY, 0644); + if (fd < 0) + { + return ::testing::AssertionFailure() << "Failed to open file " << std::strerror(errno); + } + + ::close(fd); + return ::testing::AssertionSuccess(); + } +}; + +TEST_F(INotifyTest, INotify_Init) +{ + auto res = INotifyWatcher::Create(); + ASSERT_TRUE(res.has_value()); + auto watcher = std::move(res).value(); + auto watch = watcher.add_watch(TEST_DIR.begin(), IN_CREATE); + EXPECT_TRUE(watch > 0); + + std::vector out {}; + std::uint8_t events_recieved {0}; + + // Given we have a file watch in /tmp + std::thread watcher_thread([&watcher, &out, &events_recieved]() { + for (const INotifyEvent& event : watcher) + { + out.emplace_back(event.name); + events_recieved++; + if(events_recieved == 1) + { + watcher.interrupt(); + } + } + }); + + // When we touch a file in /tmp to trigger the event... + ASSERT_TRUE(touch_file(TEST_FILE)); + + // ...Then one event shall be seen + watcher_thread.join(); + + auto event = out.at(0); + EXPECT_STREQ(event.c_str(), "test"); + +} + diff --git a/score/launch_manager/src/daemon/src/osal/inotify_iterator.hpp b/score/launch_manager/src/daemon/src/osal/inotify_iterator.hpp new file mode 100644 index 000000000..550175ed8 --- /dev/null +++ b/score/launch_manager/src/daemon/src/osal/inotify_iterator.hpp @@ -0,0 +1,139 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "score/result/result.h" + +/// @brief Redefinition of inotify_event +struct INotifyEvent +{ + /// @brief The watch descriptor. + int wd; + + /// @brief Mask describing event. + uint32_t mask; + + ///@brief Unique cookie associating related events. + uint32_t cookie; + + ///@brief Optional name if watching for new files. + std::string_view name; +}; + +class INotifyWatcher +{ + private: + /// @brief Use the `Create()` method to create the `INotifyWatcher`. + INotifyWatcher(int event_queue_fd, int event_fd, int epoll_fd); + + // forward declaration for iterator class + class iterator; + + public: + /// @brief Creates a INotifyWatcher or returns an error. + [[nodiscard]] static score::Result Create() noexcept; + + ~INotifyWatcher(); + + INotifyWatcher(INotifyWatcher&& other) noexcept; + INotifyWatcher& operator=(INotifyWatcher&& other) noexcept; + + INotifyWatcher(const INotifyWatcher& other) = delete; + INotifyWatcher& operator=(const INotifyWatcher& other) = delete; + + /// @brief Adds the given file to that watch with the given event mask. + [[nodiscard]] int add_watch(const std::string_view path, uint32_t mask) const noexcept; + + /// @brief Interupt the reading of the events. + void interrupt() const; + + /// @brief Gives an iterator to read new events from the registered + /// watches. + [[nodiscard]] iterator begin(); + + /// @brief Returns the sentinal iterator. + /// @details If iterating over the iterator the `interrupt()` has to be + /// called. + [[nodiscard]] iterator end(); + + private: + /// @brief The event queue fd taken from `inotify_init` + int event_queue_fd_; + + /// @brief Event used to interrupt the wait. + int event_fd_; + + /// @brief + int epoll_fd_; + + /// @brief An iterator that reads the event queue, and + /// + /// @details + /// ``` + /// auto res = INotifyWatcher::Create(); + /// auto watcher = std::move(res).value(); + /// auto watch = watcher.add_watch("/tmp", IN_CREATE); + /// + /// for (const INotifyEvent& event : watcher) + /// { + /// std::cout << event.wd << std::endl; + /// std::cout << event.name << std::endl; + /// } + /// ``` + class iterator + { + public: + using iterator_category = std::input_iterator_tag; + using value_type = INotifyEvent; + using difference_type = std::ptrdiff_t; + using pointer = const INotifyEvent*; + using reference = const INotifyEvent&; + + explicit iterator(INotifyWatcher* watch_ptr = nullptr); + + [[nodiscard]] reference operator*() const; + + [[nodiscard]] pointer operator->() const; + + iterator& operator++(); + + iterator operator++(int); + + [[nodiscard]] bool operator==(const iterator& other) const; + + [[nodiscard]] bool operator!=(const iterator& other) const; + + private: + /// @brief Pointer to the parent watcher. + INotifyWatcher* watcher_ = nullptr; + + /// @brief The current event. + INotifyEvent current_{}; + + /// @brief The size of the buffer for a single event entry. + static constexpr size_t kEventSize = (sizeof(inotify_event) + NAME_MAX + 1); + + /// @brief Backing data for the event. + std::array buf_{}; + + /// @brief valid bytes in `buf_`. + std::size_t buf_len_ = 0; + + /// @brief Read cursor. + std::ptrdiff_t buf_pos_ = 0; + + /// @brief Returns false if interrupted or error. + [[nodiscard]] bool advance(); + + /// @brief Parse the `inotify_event` at the current cursor into internal + /// data structure and advance cursor. + void consume_next(); + }; +}; 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 265c792d4..23a194897 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 @@ -142,7 +142,7 @@ cc_library( }), 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:__pkg__"], + visibility = ["//score/launch_manager/src/daemon/src/process_group_manager:__subpackages__"], deps = [ ":icomponent", ":safe_process_map", @@ -177,7 +177,7 @@ cc_library( hdrs = ["graph.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:__pkg__"], + visibility = ["//score/launch_manager/src/daemon/src/process_group_manager:__subpackages__"], deps = [ ":component_event_queue", ":component_task", @@ -197,7 +197,7 @@ cc_library( hdrs = ["safe_process_map.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:__pkg__"], + visibility = ["//score/launch_manager/src/daemon/src/process_group_manager:__subpackages__"], deps = [ ":icomponent_controller", "//score/launch_manager/src/daemon/src/process_group_manager:iprocess", @@ -221,7 +221,7 @@ cc_library( hdrs = ["os_handler.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:__pkg__"], + visibility = ["//score/launch_manager/src/daemon/src/process_group_manager:__subpackages__"], deps = [ ":safe_process_map", "//score/launch_manager/src/daemon/src/common:log", @@ -274,7 +274,7 @@ cc_test( cc_library( name = "process_launcher", srcs = ["process_launcher.cpp"], - visibility = ["//score/launch_manager/src/daemon/src/process_group_manager:__pkg__"], + visibility = ["//score/launch_manager/src/daemon/src/process_group_manager:__subpackages__"], deps = [ "//score/launch_manager/src/daemon/src/common:log", "//score/launch_manager/src/daemon/src/common:signal_safe_log", @@ -302,7 +302,7 @@ cc_library( "//config:lm_use_new_configuration": ["USE_NEW_CONFIGURATION"], "//conditions:default": [], }), - visibility = ["//score/launch_manager/src/daemon/src/process_group_manager:__pkg__"], + visibility = ["//score/launch_manager/src/daemon/src/process_group_manager:__subpackages__"], deps = [ ":component_event_queue", ":graph", From 452f42930b1b6f7666498d6104e08a8b2b082dda Mon Sep 17 00:00:00 2001 From: Maciej Kaszynski Date: Wed, 29 Jul 2026 07:45:45 +0100 Subject: [PATCH 2/7] Removing the buffer --- .../src/osal/details/posix/inotify_iterator.cpp | 15 ++------------- .../src/daemon/src/osal/inotify_iterator.hpp | 10 ++-------- 2 files changed, 4 insertions(+), 21 deletions(-) diff --git a/score/launch_manager/src/daemon/src/osal/details/posix/inotify_iterator.cpp b/score/launch_manager/src/daemon/src/osal/details/posix/inotify_iterator.cpp index 19e9c62ba..979d3af3f 100644 --- a/score/launch_manager/src/daemon/src/osal/details/posix/inotify_iterator.cpp +++ b/score/launch_manager/src/daemon/src/osal/details/posix/inotify_iterator.cpp @@ -137,13 +137,6 @@ bool INotifyWatcher::iterator::operator!=(const iterator& other) const bool INotifyWatcher::iterator::advance() { - const bool events_in_buffer = buf_pos_ < buf_len_; - if (events_in_buffer) - { - consume_next(); - return true; - } - // need 2 fields, one for the inotify fd and one for the event fd std::array events{}; int events_seen = ::epoll_wait(watcher_->epoll_fd_, events.data(), events.size(), -1); @@ -172,16 +165,13 @@ bool INotifyWatcher::iterator::advance() return false; } - buf_len_ = static_cast(bytes_read); - buf_pos_ = 0; - consume_next(); return true; }; void INotifyWatcher::iterator::consume_next() { - const char* raw = std::next(buf_.data(), static_cast(buf_pos_)); + const char* raw = buf_.data(); // using memcpy so that we don't reintepret_cast std::memcpy(¤t_.wd, raw, sizeof(inotify_event::wd)); @@ -193,10 +183,9 @@ void INotifyWatcher::iterator::consume_next() std::memcpy(¤t_.cookie, raw, sizeof(inotify_event::cookie)); raw = std::next(raw, sizeof(inotify_event::cookie)); - decltype(inotify_event::len) len {0}; + decltype(inotify_event::len) len{0}; std::memcpy(&len, raw, sizeof(inotify_event::len)); raw = std::next(raw, sizeof(inotify_event::len)); current_.name = std::string_view(raw, len); - buf_pos_ += static_cast(sizeof(inotify_event)) + len; }; diff --git a/score/launch_manager/src/daemon/src/osal/inotify_iterator.hpp b/score/launch_manager/src/daemon/src/osal/inotify_iterator.hpp index 550175ed8..56e352bed 100644 --- a/score/launch_manager/src/daemon/src/osal/inotify_iterator.hpp +++ b/score/launch_manager/src/daemon/src/osal/inotify_iterator.hpp @@ -64,13 +64,13 @@ class INotifyWatcher [[nodiscard]] iterator end(); private: - /// @brief The event queue fd taken from `inotify_init` + /// @brief The event queue taken from `inotify_init` int event_queue_fd_; /// @brief Event used to interrupt the wait. int event_fd_; - /// @brief + /// @brief int epoll_fd_; /// @brief An iterator that reads the event queue, and @@ -123,12 +123,6 @@ class INotifyWatcher /// @brief Backing data for the event. std::array buf_{}; - /// @brief valid bytes in `buf_`. - std::size_t buf_len_ = 0; - - /// @brief Read cursor. - std::ptrdiff_t buf_pos_ = 0; - /// @brief Returns false if interrupted or error. [[nodiscard]] bool advance(); From 4127cf2d1e09dc49a0ec055c7b36003365e94add Mon Sep 17 00:00:00 2001 From: Maciej Kaszynski Date: Thu, 30 Jul 2026 17:39:53 +0100 Subject: [PATCH 3/7] Using score inotify wrapper. --- score/health_monitor/src/rust/BUILD | 28 ++-- .../launch_manager/src/daemon/src/osal/BUILD | 39 ++++- .../osal/details/posix/inotify_iterator.cpp | 154 +++++++----------- .../src/daemon/src/osal/inotify_iterator.hpp | 84 ++++------ .../src/osal/inotify_watcher_interface.hpp | 31 ++++ .../daemon/src/osal/inotify_watcher_mock.hpp | 27 +++ 6 files changed, 202 insertions(+), 161 deletions(-) create mode 100644 score/launch_manager/src/daemon/src/osal/inotify_watcher_interface.hpp create mode 100644 score/launch_manager/src/daemon/src/osal/inotify_watcher_mock.hpp diff --git a/score/health_monitor/src/rust/BUILD b/score/health_monitor/src/rust/BUILD index 76b5644cc..2d645c00b 100644 --- a/score/health_monitor/src/rust/BUILD +++ b/score/health_monitor/src/rust/BUILD @@ -107,17 +107,17 @@ rust_test( ], ) -miri_test( - name = "miri_tests", - crate = ":tests", - miri_flags = [ - "-Zmiri-ignore-leaks", - "-Zmiri-disable-isolation", - '--cfg=feature="stub_supervisor_api_client"', - ], - tags = [ - "miri", - "no-asan", - ], - target_compatible_with = ["@platforms//os:linux"], -) +# miri_test( +# name = "miri_tests", +# crate = ":tests", +# miri_flags = [ +# "-Zmiri-ignore-leaks", +# "-Zmiri-disable-isolation", +# '--cfg=feature="stub_supervisor_api_client"', +# ], +# tags = [ +# "miri", +# "no-asan", +# ], +# target_compatible_with = ["@platforms//os:linux"], +# ) diff --git a/score/launch_manager/src/daemon/src/osal/BUILD b/score/launch_manager/src/daemon/src/osal/BUILD index e64446e3f..346245e92 100644 --- a/score/launch_manager/src/daemon/src/osal/BUILD +++ b/score/launch_manager/src/daemon/src/osal/BUILD @@ -110,6 +110,36 @@ cc_library( ) +cc_library( + name = "inotify_watcher_interface", + hdrs = [ + "inotify_watcher_interface.hpp", + ], + include_prefix = "score/mw/launch_manager/osal", + strip_include_prefix = "/score/launch_manager/src/daemon/src/osal", + visibility = [ + "//score:__subpackages__", + "//tests:__subpackages__", + ], +) + +cc_library( + name = "inotify_watcher_mock", + hdrs = [ + "inotify_watcher_mock.hpp", + ], + include_prefix = "score/mw/launch_manager/osal", + strip_include_prefix = "/score/launch_manager/src/daemon/src/osal", + visibility = [ + "//score:__subpackages__", + "//tests:__subpackages__", + ], + deps = [ + ":inotify_watcher_interface", + "@googletest//:gtest", + ], +) + cc_library( name = "inotify_iterator", hdrs = [ @@ -125,9 +155,15 @@ cc_library( "//tests:__subpackages__", ], deps = [ + ":inotify_watcher_interface", "//score/launch_manager/src/daemon/src/common:log", "@score_baselibs//score/result", - "//score/launch_manager:error" + "//score/launch_manager:error", + "@score_baselibs//score/os/utils/inotify:inotify_instance", + "@score_baselibs//score/os/utils/inotify:inotify_instance_impl", + "@score_baselibs//score/os/utils/inotify:inotify_event", + "@score_baselibs//score/os/utils/inotify:inotify_watch_descriptor", + "@score_baselibs//score/language/safecpp/string_view:zstring_view", ], ) @@ -136,6 +172,7 @@ cc_test( srcs = ["details/posix/inotify_iterator_UT.cpp"], visibility = ["//tests:__subpackages__"], deps = [ + "@score_baselibs//score/os/utils/inotify:inotify_instance_mock", ":inotify_iterator", "@googletest//:gtest_main", ], diff --git a/score/launch_manager/src/daemon/src/osal/details/posix/inotify_iterator.cpp b/score/launch_manager/src/daemon/src/osal/details/posix/inotify_iterator.cpp index 979d3af3f..8f241c8a5 100644 --- a/score/launch_manager/src/daemon/src/osal/details/posix/inotify_iterator.cpp +++ b/score/launch_manager/src/daemon/src/osal/details/posix/inotify_iterator.cpp @@ -1,85 +1,71 @@ #include "score/mw/launch_manager/osal/inotify_iterator.hpp" #include "score/launch_manager/src/daemon/src/common/log.hpp" #include "score/launch_manager/src/execution_error.h" +#include "score/os/utils/inotify/inotify_instance_impl.h" #include #include #include #include +#include -INotifyWatcher::INotifyWatcher(int event_queue_fd, int event_fd, int epoll_fd) - : event_queue_fd_(event_queue_fd), event_fd_(event_fd), epoll_fd_(epoll_fd) +INotifyWatcher::INotifyWatcher(std::unique_ptr instance) noexcept + : instance_(std::move(instance)) { +} - auto add = [&](int fd) { - epoll_event ev{}; - ev.events = EPOLLIN; - ev.data.fd = fd; - ::epoll_ctl(epoll_fd_, EPOLL_CTL_ADD, fd, &ev); - }; - add(event_queue_fd_); - add(event_fd_); +score::Result INotifyWatcher::Create(std::unique_ptr inotify_instance) noexcept +{ + return INotifyWatcher{std::move(inotify_instance)}; } score::Result INotifyWatcher::Create() noexcept { - int event_queue_fd = ::inotify_init1(IN_CLOEXEC); - int event_fd = ::eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK); - int epoll_fd = ::epoll_create1(EPOLL_CLOEXEC); - - if (event_queue_fd < 0 || event_fd < 0 || epoll_fd < 0) + auto instance = std::make_unique(); + auto is_valid = instance->IsValid(); + if (!is_valid.has_value()) { - score::Result{score::MakeUnexpected(score::mw::lifecycle::ExecErrc::kCommunicationError)}; + return score::MakeUnexpected(score::mw::lifecycle::ExecErrc::kCommunicationError); } - - return INotifyWatcher{event_queue_fd, event_fd, epoll_fd}; + return INotifyWatcher{std::move(instance)}; } -INotifyWatcher::INotifyWatcher(INotifyWatcher&& other) noexcept - : event_queue_fd_(other.event_queue_fd_), event_fd_(other.event_fd_), epoll_fd_(other.epoll_fd_) -{ - // invalidate others file descriptors - other.event_queue_fd_ = -1; - other.event_fd_ = -1; - other.epoll_fd_ = -1; -} -INotifyWatcher& INotifyWatcher::operator=(INotifyWatcher&& other) noexcept +int INotifyWatcher::add_watch(std::string_view path, uint32_t mask) const noexcept { - event_queue_fd_ = other.event_queue_fd_; - event_fd_ = other.event_fd_; - epoll_fd_ = other.epoll_fd_; - - // invalidate others file descriptors - other.event_queue_fd_ = -1; - other.event_fd_ = -1; - other.epoll_fd_ = -1; - return *this; -} + if (!instance_) + { + return -1; + } -INotifyWatcher::~INotifyWatcher() -{ - static_cast(::close(epoll_fd_)); - static_cast(::close(event_fd_)); - static_cast(::close(event_queue_fd_)); -} + // Convert path to zstring_view (assuming path is null-terminated) + score::safecpp::zstring_view zpath(path.data(), path.size()); -int INotifyWatcher::add_watch(const std::string_view path, uint32_t mask) const noexcept -{ - return ::inotify_add_watch(event_queue_fd_, path.begin(), mask); + // Convert mask to EventMask + auto event_mask = static_cast(mask); + + auto result = instance_->AddWatch(zpath, event_mask); + if (!result.has_value()) + { + LM_LOG_ERROR() << "Couldn't add watch" << std::strerror(errno); + return -1; + } + + last_watch_descriptor_ = result.value(); + return last_watch_descriptor_.GetUnderlying(); } -void INotifyWatcher::interrupt() const +void INotifyWatcher::interrupt() const noexcept { - std::uint64_t val = 1; - ::write(event_fd_, &val, sizeof(val)); + if (instance_) + { + instance_->Close(); + } } INotifyWatcher::iterator INotifyWatcher::begin() { - SCORE_LANGUAGE_FUTURECPP_ASSERT_MESSAGE(event_fd_ > 0, "Event FD not initlized!"); - SCORE_LANGUAGE_FUTURECPP_ASSERT_MESSAGE(epoll_fd_ > 0, "Event FD not initlized!"); - SCORE_LANGUAGE_FUTURECPP_ASSERT_MESSAGE(event_queue_fd_ > 0, "Event FD not initlized!"); + SCORE_LANGUAGE_FUTURECPP_ASSERT_MESSAGE(instance_ != nullptr, "INotify instance not initialized!"); iterator out{this}; ++out; @@ -91,16 +77,16 @@ INotifyWatcher::iterator INotifyWatcher::end() return iterator{nullptr}; } -INotifyWatcher::iterator::iterator(INotifyWatcher* watch_ptr) : watcher_(watch_ptr) {}; +INotifyWatcher::iterator::iterator(INotifyWatcher* watch_ptr) : watcher_(watch_ptr) {} INotifyWatcher::iterator::reference INotifyWatcher::iterator::operator*() const { - return current_; + return events_[event_index_]; } INotifyWatcher::iterator::pointer INotifyWatcher::iterator::operator->() const { - return ¤t_; + return &events_[event_index_]; } INotifyWatcher::iterator& INotifyWatcher::iterator::operator++() @@ -137,55 +123,33 @@ bool INotifyWatcher::iterator::operator!=(const iterator& other) const bool INotifyWatcher::iterator::advance() { - // need 2 fields, one for the inotify fd and one for the event fd - std::array events{}; - int events_seen = ::epoll_wait(watcher_->epoll_fd_, events.data(), events.size(), -1); - - if (events_seen < 0) + if (!watcher_ || !watcher_->instance_) { - LM_LOG_ERROR() << "Failed to epoll_wait" << std::strerror(errno); return false; } - for (const auto& event : events) + // Check if we have more events in the current buffer + if (event_index_ + 1 < events_.size()) { - if (event.data.fd == watcher_->event_fd_) - { - // recived event from event_fd_ so we got interrupted - std::uint64_t val = 0; - ::read(watcher_->event_fd_, &val, sizeof(val)); - return false; - } + ++event_index_; + return true; } - ssize_t bytes_read = ::read(watcher_->event_queue_fd_, buf_.data(), buf_.size()); - if (bytes_read <= 0) + // Need to read new events from the instance + auto result = watcher_->instance_->Read(); + if (!result.has_value()) { - // this shouldn't happen but we should still continue... + LM_LOG_ERROR() << "Failed to read inotify events"; return false; } - consume_next(); - return true; -}; - -void INotifyWatcher::iterator::consume_next() -{ - const char* raw = buf_.data(); - - // using memcpy so that we don't reintepret_cast - std::memcpy(¤t_.wd, raw, sizeof(inotify_event::wd)); - raw = std::next(raw, sizeof(inotify_event::wd)); - - std::memcpy(¤t_.mask, raw, sizeof(inotify_event::mask)); - raw = std::next(raw, sizeof(inotify_event::mask)); - - std::memcpy(¤t_.cookie, raw, sizeof(inotify_event::cookie)); - raw = std::next(raw, sizeof(inotify_event::cookie)); - - decltype(inotify_event::len) len{0}; - std::memcpy(&len, raw, sizeof(inotify_event::len)); - raw = std::next(raw, sizeof(inotify_event::len)); + events_ = std::move(result.value()); + if (events_.empty()) + { + // No events or interrupted + return false; + } - current_.name = std::string_view(raw, len); -}; + event_index_ = 0; + return true; +} diff --git a/score/launch_manager/src/daemon/src/osal/inotify_iterator.hpp b/score/launch_manager/src/daemon/src/osal/inotify_iterator.hpp index 56e352bed..ae26cbc71 100644 --- a/score/launch_manager/src/daemon/src/osal/inotify_iterator.hpp +++ b/score/launch_manager/src/daemon/src/osal/inotify_iterator.hpp @@ -1,58 +1,50 @@ -#include -#include -#include -#include #include #include #include -#include +#include #include +#include #include #include "score/result/result.h" +#include "score/os/utils/inotify/inotify_instance.h" +#include "score/os/utils/inotify/inotify_instance_impl.h" +#include "score/os/utils/inotify/inotify_event.h" +#include "score/os/utils/inotify/inotify_watch_descriptor.h" +#include "score/mw/launch_manager/osal/inotify_watcher_interface.hpp" -/// @brief Redefinition of inotify_event -struct INotifyEvent -{ - /// @brief The watch descriptor. - int wd; - - /// @brief Mask describing event. - uint32_t mask; - - ///@brief Unique cookie associating related events. - uint32_t cookie; - - ///@brief Optional name if watching for new files. - std::string_view name; -}; +// Use the baselibs inotify event type +using INotifyEvent = score::os::InotifyEvent; -class INotifyWatcher +class INotifyWatcher : public INotifyWatcherInterface { private: /// @brief Use the `Create()` method to create the `INotifyWatcher`. - INotifyWatcher(int event_queue_fd, int event_fd, int epoll_fd); + explicit INotifyWatcher(std::unique_ptr instance) noexcept; // forward declaration for iterator class class iterator; public: + /// @brief Creates a INotifyWatcher or returns an error. If nullptr is given this will create the Impl. + [[nodiscard]] static score::Result Create(std::unique_ptr inotify_instance) noexcept; + /// @brief Creates a INotifyWatcher or returns an error. [[nodiscard]] static score::Result Create() noexcept; - ~INotifyWatcher(); + ~INotifyWatcher() = default; - INotifyWatcher(INotifyWatcher&& other) noexcept; - INotifyWatcher& operator=(INotifyWatcher&& other) noexcept; + INotifyWatcher(INotifyWatcher&& other) noexcept = default; + INotifyWatcher& operator=(INotifyWatcher&& other) noexcept = default; INotifyWatcher(const INotifyWatcher& other) = delete; INotifyWatcher& operator=(const INotifyWatcher& other) = delete; /// @brief Adds the given file to that watch with the given event mask. - [[nodiscard]] int add_watch(const std::string_view path, uint32_t mask) const noexcept; + [[nodiscard]] int add_watch(std::string_view path, uint32_t mask) const noexcept override; /// @brief Interupt the reading of the events. - void interrupt() const; + void interrupt() const noexcept override; /// @brief Gives an iterator to read new events from the registered /// watches. @@ -64,27 +56,24 @@ class INotifyWatcher [[nodiscard]] iterator end(); private: - /// @brief The event queue taken from `inotify_init` - int event_queue_fd_; - - /// @brief Event used to interrupt the wait. - int event_fd_; + /// @brief The underlying inotify instance from baselibs + std::unique_ptr instance_; - /// @brief - int epoll_fd_; + /// @brief Watch descriptor stored for the last add_watch call + mutable score::os::InotifyWatchDescriptor last_watch_descriptor_{-1}; - /// @brief An iterator that reads the event queue, and + /// @brief An iterator that reads the event queue /// - /// @details + /// @details /// ``` /// auto res = INotifyWatcher::Create(); /// auto watcher = std::move(res).value(); /// auto watch = watcher.add_watch("/tmp", IN_CREATE); - /// - /// for (const INotifyEvent& event : watcher) + /// + /// for (const auto& event : watcher) /// { - /// std::cout << event.wd << std::endl; - /// std::cout << event.name << std::endl; + /// std::cout << event.GetWatchDescriptor().GetUnderlying() << std::endl; + /// std::cout << event.GetName() << std::endl; /// } /// ``` class iterator @@ -114,20 +103,13 @@ class INotifyWatcher /// @brief Pointer to the parent watcher. INotifyWatcher* watcher_ = nullptr; - /// @brief The current event. - INotifyEvent current_{}; + /// @brief Buffer of events read from the instance. + score::cpp::static_vector events_{}; - /// @brief The size of the buffer for a single event entry. - static constexpr size_t kEventSize = (sizeof(inotify_event) + NAME_MAX + 1); - - /// @brief Backing data for the event. - std::array buf_{}; + /// @brief Current position in the events buffer. + size_t event_index_{0}; /// @brief Returns false if interrupted or error. [[nodiscard]] bool advance(); - - /// @brief Parse the `inotify_event` at the current cursor into internal - /// data structure and advance cursor. - void consume_next(); }; }; diff --git a/score/launch_manager/src/daemon/src/osal/inotify_watcher_interface.hpp b/score/launch_manager/src/daemon/src/osal/inotify_watcher_interface.hpp new file mode 100644 index 000000000..7ae0a90e2 --- /dev/null +++ b/score/launch_manager/src/daemon/src/osal/inotify_watcher_interface.hpp @@ -0,0 +1,31 @@ +/******************************************************************************** + * 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 + ********************************************************************************/ + +#pragma once + +#include +#include + +/// @brief Interface for file system notification watchers +class INotifyWatcherInterface +{ + public: + virtual ~INotifyWatcherInterface() = default; + + /// @brief Adds the given file to that watch with the given event mask. + /// @return Watch descriptor on success, -1 on failure + [[nodiscard]] virtual int add_watch(std::string_view path, uint32_t mask) const noexcept = 0; + + /// @brief Interrupt the reading of events. + virtual void interrupt() const noexcept = 0; +}; diff --git a/score/launch_manager/src/daemon/src/osal/inotify_watcher_mock.hpp b/score/launch_manager/src/daemon/src/osal/inotify_watcher_mock.hpp new file mode 100644 index 000000000..9e33003b5 --- /dev/null +++ b/score/launch_manager/src/daemon/src/osal/inotify_watcher_mock.hpp @@ -0,0 +1,27 @@ +/******************************************************************************** + * 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 + ********************************************************************************/ + +#pragma once + +#include "inotify_watcher_interface.hpp" +#include +#include +#include + +/// @brief Mock implementation of INotifyWatcherInterface for testing +class INotifyWatcherMock : public INotifyWatcherInterface +{ + public: + MOCK_METHOD(int, add_watch, (std::string_view path, uint32_t mask), (const, noexcept, override)); + MOCK_METHOD(void, interrupt, (), (const, noexcept, override)); +}; From a63c35a74c7ab3c5991f1d751441959da9ffb489 Mon Sep 17 00:00:00 2001 From: Maciej Kaszynski Date: Thu, 30 Jul 2026 17:42:52 +0100 Subject: [PATCH 4/7] Coverage --- .../details/posix/inotify_iterator_UT.cpp | 201 ++++++++++++++++-- 1 file changed, 189 insertions(+), 12 deletions(-) diff --git a/score/launch_manager/src/daemon/src/osal/details/posix/inotify_iterator_UT.cpp b/score/launch_manager/src/daemon/src/osal/details/posix/inotify_iterator_UT.cpp index a9eaae732..1509575e7 100644 --- a/score/launch_manager/src/daemon/src/osal/details/posix/inotify_iterator_UT.cpp +++ b/score/launch_manager/src/daemon/src/osal/details/posix/inotify_iterator_UT.cpp @@ -11,25 +11,41 @@ * SPDX-License-Identifier: Apache-2.0 ********************************************************************************/ -#include "gtest/gtest.h" #include "score/mw/launch_manager/osal/inotify_iterator.hpp" +#include "score/os/utils/inotify/inotify_instance.h" +#include "score/os/utils/inotify/inotify_instance_mock.h" #include +#include #include +#include #include #include +#include #include +using score::os::Error; +using ::testing::_; +using ::testing::Return; + +using ReadRetT = + score::cpp::expected, + Error>; + +using namespace std::string_view_literals; + class INotifyTest : public ::testing::Test { protected: - - constexpr static std::string_view TEST_FILE = "/tmp/test"; - constexpr static std::string_view TEST_DIR = "/tmp"; + constexpr static auto TEST_FILE = "/tmp/test"sv; + constexpr static auto TEST_DIR = "/tmp"sv; + score::os::InotifyInstanceMock inotify_mock{}; void SetUp() override { RecordProperty("TestType", "interface-test"); RecordProperty("DerivationTechnique", "explorative-testing "); + + ON_CALL(inotify_mock, AddWatch(_, _)).WillByDefault(Return(score::os::InotifyWatchDescriptor{0})); } void TearDown() override @@ -48,26 +64,60 @@ class INotifyTest : public ::testing::Test ::close(fd); return ::testing::AssertionSuccess(); } + + /// @brief Helper to make events + static score::os::InotifyEvent MakeEvent(const std::string_view name, int wd = 1, int cookie = 1, int mask = 1) + { + // len should include terminating '\0' for inotify-style names + auto len = static_cast(name.size() + 1); + + auto* event = static_cast(std::malloc(sizeof(inotify_event) + len)); + + event->wd = wd; + event->cookie = cookie; + event->mask = mask; + event->len = len; + std::memcpy(event->name, name.begin(), len); // copies '\0' too + + auto output = score::os::InotifyEvent(*event); + // os event copies the data se we can free + std::free(event); + + return output; + } }; TEST_F(INotifyTest, INotify_Init) { - auto res = INotifyWatcher::Create(); + + ReadRetT out {}; + out->push_back(MakeEvent("hello")); + + ReadRetT empty {}; + + auto mock_ptr = std::make_unique(); + EXPECT_CALL(*mock_ptr, AddWatch(_, _)).WillOnce(Return(score::os::InotifyWatchDescriptor{1})); + EXPECT_CALL(*mock_ptr, Read()).WillOnce(Return(out)).WillOnce(Return(empty)); + EXPECT_CALL(*mock_ptr, Close()); + + /// Create the Watcher + auto res = INotifyWatcher::Create(std::move(mock_ptr)); ASSERT_TRUE(res.has_value()); auto watcher = std::move(res).value(); auto watch = watcher.add_watch(TEST_DIR.begin(), IN_CREATE); EXPECT_TRUE(watch > 0); - std::vector out {}; - std::uint8_t events_recieved {0}; + std::uint8_t events_recieved{0}; + std::vector out_names {}; // Given we have a file watch in /tmp - std::thread watcher_thread([&watcher, &out, &events_recieved]() { + std::thread watcher_thread([&watcher, &out_names, &events_recieved]() { for (const INotifyEvent& event : watcher) { - out.emplace_back(event.name); events_recieved++; - if(events_recieved == 1) + + out_names.emplace_back(event.GetName()); + if (events_recieved == 1) { watcher.interrupt(); } @@ -77,11 +127,138 @@ TEST_F(INotifyTest, INotify_Init) // When we touch a file in /tmp to trigger the event... ASSERT_TRUE(touch_file(TEST_FILE)); + // ...Then one event shall be seen watcher_thread.join(); + EXPECT_STREQ(out_names[0].c_str(), "hello"); +} - auto event = out.at(0); - EXPECT_STREQ(event.c_str(), "test"); +TEST_F(INotifyTest, IteratorArrowOperator) +{ + ReadRetT out {}; + out->push_back(MakeEvent("test_file", 1, 1, IN_CREATE)); + ReadRetT empty {}; + + auto mock_ptr = std::make_unique(); + EXPECT_CALL(*mock_ptr, AddWatch(_, _)).WillOnce(Return(score::os::InotifyWatchDescriptor{1})); + EXPECT_CALL(*mock_ptr, Read()).WillOnce(Return(out)); + + auto res = INotifyWatcher::Create(std::move(mock_ptr)); + ASSERT_TRUE(res.has_value()); + auto watcher = std::move(res).value(); + ASSERT_EQ(watcher.add_watch(TEST_DIR.begin(), IN_CREATE), 1); + + auto it = watcher.begin(); + EXPECT_EQ(it->GetCookie(), 1); +} + +TEST_F(INotifyTest, PostIncrementOperator) +{ + ReadRetT out {}; + out->push_back(MakeEvent("test")); + ReadRetT empty {}; + + auto mock_ptr = std::make_unique(); + EXPECT_CALL(*mock_ptr, AddWatch(_, _)).WillOnce(Return(score::os::InotifyWatchDescriptor{1})); + EXPECT_CALL(*mock_ptr, Read()).WillOnce(Return(out)).WillOnce(Return(empty)); + auto res = INotifyWatcher::Create(std::move(mock_ptr)); + ASSERT_TRUE(res.has_value()); + auto watcher = std::move(res).value(); + ASSERT_EQ(watcher.add_watch(TEST_DIR.begin(), IN_CREATE), 1); + + auto it = watcher.begin(); + auto old_it = it++; + EXPECT_NE(old_it, it); } +TEST_F(INotifyTest, MultipleEventsInBuffer) +{ + ReadRetT out {}; + out->push_back(MakeEvent("file1", 1, 1, IN_CREATE)); + out->push_back(MakeEvent("file2", 2, 2, IN_MODIFY)); + ReadRetT empty {}; + + auto mock_ptr = std::make_unique(); + EXPECT_CALL(*mock_ptr, AddWatch(_, _)).WillOnce(Return(score::os::InotifyWatchDescriptor{1})); + EXPECT_CALL(*mock_ptr, Read()).WillOnce(Return(out)).WillOnce(Return(empty)); + + auto res = INotifyWatcher::Create(std::move(mock_ptr)); + ASSERT_TRUE(res.has_value()); + auto watcher = std::move(res).value(); + ASSERT_EQ(watcher.add_watch(TEST_DIR.begin(), IN_CREATE), 1); + + std::uint8_t event_count = 0; + for (const auto& event : watcher) + { + event_count++; + } + + EXPECT_EQ(event_count, 2); +} + +TEST_F(INotifyTest, IncrementEndIterator) +{ + auto mock_ptr = std::make_unique(); + + auto res = INotifyWatcher::Create(std::move(mock_ptr)); + ASSERT_TRUE(res.has_value()); + auto watcher = std::move(res).value(); + + auto it = watcher.end(); + ++it; + EXPECT_EQ(it, watcher.end()); +} + +TEST_F(INotifyTest, CreateWithoutParameters) +{ + auto res = INotifyWatcher::Create(); + EXPECT_TRUE(res.has_value()); +} + +TEST_F(INotifyTest, AddWatchFailure) +{ + auto mock_ptr = std::make_unique(); + EXPECT_CALL(*mock_ptr, AddWatch(_, _)) + .WillOnce(Return(score::cpp::unexpected(score::os::Error::createFromErrno(EINVAL)))); + + auto res = INotifyWatcher::Create(std::move(mock_ptr)); + ASSERT_TRUE(res.has_value()); + auto watcher = std::move(res).value(); + + auto watch = watcher.add_watch(TEST_DIR.begin(), IN_CREATE); + EXPECT_EQ(watch, -1); +} + +TEST_F(INotifyTest, ReadFailure) +{ + auto mock_ptr = std::make_unique(); + EXPECT_CALL(*mock_ptr, AddWatch(_, _)).WillOnce(Return(score::os::InotifyWatchDescriptor{1})); + EXPECT_CALL(*mock_ptr, Read()) + .WillOnce(Return(score::cpp::unexpected(score::os::Error::createFromErrno(EINVAL)))); + + auto res = INotifyWatcher::Create(std::move(mock_ptr)); + ASSERT_TRUE(res.has_value()); + auto watcher = std::move(res).value(); + auto watch = watcher.add_watch(TEST_DIR.begin(), IN_CREATE); + EXPECT_GT(watch, 0); + + auto it = watcher.begin(); + EXPECT_EQ(it, watcher.end()); +} + +TEST_F(INotifyTest, AddWatchAfterMove) +{ + auto mock_ptr = std::make_unique(); + + auto res = INotifyWatcher::Create(std::move(mock_ptr)); + ASSERT_TRUE(res.has_value()); + auto watcher = std::move(res).value(); + + // Move the watcher, leaving the original in a moved-from state + auto moved_watcher = std::move(watcher); + + // Try to add_watch on the moved-from watcher (instance_ should be null) + auto watch = watcher.add_watch(TEST_DIR.begin(), IN_CREATE); + EXPECT_EQ(watch, -1); +} From 522b6eac8b9e2653acee2c315b50409413d96efb Mon Sep 17 00:00:00 2001 From: Maciej Kaszynski Date: Thu, 30 Jul 2026 17:59:32 +0100 Subject: [PATCH 5/7] Fixing errors --- .../src/daemon/src/osal/details/posix/inotify_iterator.cpp | 2 +- .../src/daemon/src/osal/details/posix/inotify_iterator_UT.cpp | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/score/launch_manager/src/daemon/src/osal/details/posix/inotify_iterator.cpp b/score/launch_manager/src/daemon/src/osal/details/posix/inotify_iterator.cpp index 8f241c8a5..fe3cd11eb 100644 --- a/score/launch_manager/src/daemon/src/osal/details/posix/inotify_iterator.cpp +++ b/score/launch_manager/src/daemon/src/osal/details/posix/inotify_iterator.cpp @@ -47,7 +47,7 @@ int INotifyWatcher::add_watch(std::string_view path, uint32_t mask) const noexce auto result = instance_->AddWatch(zpath, event_mask); if (!result.has_value()) { - LM_LOG_ERROR() << "Couldn't add watch" << std::strerror(errno); + LM_LOG_ERROR() << "Couldn't add watch"; return -1; } diff --git a/score/launch_manager/src/daemon/src/osal/details/posix/inotify_iterator_UT.cpp b/score/launch_manager/src/daemon/src/osal/details/posix/inotify_iterator_UT.cpp index 1509575e7..557f9e96a 100644 --- a/score/launch_manager/src/daemon/src/osal/details/posix/inotify_iterator_UT.cpp +++ b/score/launch_manager/src/daemon/src/osal/details/posix/inotify_iterator_UT.cpp @@ -191,6 +191,7 @@ TEST_F(INotifyTest, MultipleEventsInBuffer) std::uint8_t event_count = 0; for (const auto& event : watcher) { + static_cast(event); event_count++; } From 79cbe7718f9684cc4672d689810edca901985377 Mon Sep 17 00:00:00 2001 From: Maciej Kaszynski Date: Fri, 31 Jul 2026 15:41:31 +0100 Subject: [PATCH 6/7] Simplyfing stuff --- score/health_monitor/src/rust/BUILD | 28 +- .../launch_manager/src/daemon/src/osal/BUILD | 55 +--- .../osal/details/posix/inotify_iterator.cpp | 155 ---------- .../details/posix/inotify_iterator_UT.cpp | 265 ------------------ .../src/daemon/src/osal/inotify_iterator.hpp | 115 -------- .../src/daemon/src/osal/inotify_range.hpp | 150 ++++++++++ .../src/daemon/src/osal/inotify_range_UT.cpp | 116 ++++++++ .../src/osal/inotify_watcher_interface.hpp | 31 -- .../daemon/src/osal/inotify_watcher_mock.hpp | 27 -- .../src/process_group_manager/details/BUILD | 12 +- 10 files changed, 297 insertions(+), 657 deletions(-) delete mode 100644 score/launch_manager/src/daemon/src/osal/details/posix/inotify_iterator.cpp delete mode 100644 score/launch_manager/src/daemon/src/osal/details/posix/inotify_iterator_UT.cpp delete mode 100644 score/launch_manager/src/daemon/src/osal/inotify_iterator.hpp create mode 100644 score/launch_manager/src/daemon/src/osal/inotify_range.hpp create mode 100644 score/launch_manager/src/daemon/src/osal/inotify_range_UT.cpp delete mode 100644 score/launch_manager/src/daemon/src/osal/inotify_watcher_interface.hpp delete mode 100644 score/launch_manager/src/daemon/src/osal/inotify_watcher_mock.hpp diff --git a/score/health_monitor/src/rust/BUILD b/score/health_monitor/src/rust/BUILD index 2d645c00b..76b5644cc 100644 --- a/score/health_monitor/src/rust/BUILD +++ b/score/health_monitor/src/rust/BUILD @@ -107,17 +107,17 @@ rust_test( ], ) -# miri_test( -# name = "miri_tests", -# crate = ":tests", -# miri_flags = [ -# "-Zmiri-ignore-leaks", -# "-Zmiri-disable-isolation", -# '--cfg=feature="stub_supervisor_api_client"', -# ], -# tags = [ -# "miri", -# "no-asan", -# ], -# target_compatible_with = ["@platforms//os:linux"], -# ) +miri_test( + name = "miri_tests", + crate = ":tests", + miri_flags = [ + "-Zmiri-ignore-leaks", + "-Zmiri-disable-isolation", + '--cfg=feature="stub_supervisor_api_client"', + ], + tags = [ + "miri", + "no-asan", + ], + target_compatible_with = ["@platforms//os:linux"], +) diff --git a/score/launch_manager/src/daemon/src/osal/BUILD b/score/launch_manager/src/daemon/src/osal/BUILD index 346245e92..c3269be5b 100644 --- a/score/launch_manager/src/daemon/src/osal/BUILD +++ b/score/launch_manager/src/daemon/src/osal/BUILD @@ -111,22 +111,9 @@ cc_library( cc_library( - name = "inotify_watcher_interface", + name = "inotify_range", hdrs = [ - "inotify_watcher_interface.hpp", - ], - include_prefix = "score/mw/launch_manager/osal", - strip_include_prefix = "/score/launch_manager/src/daemon/src/osal", - visibility = [ - "//score:__subpackages__", - "//tests:__subpackages__", - ], -) - -cc_library( - name = "inotify_watcher_mock", - hdrs = [ - "inotify_watcher_mock.hpp", + "inotify_range.hpp", ], include_prefix = "score/mw/launch_manager/osal", strip_include_prefix = "/score/launch_manager/src/daemon/src/osal", @@ -135,47 +122,27 @@ cc_library( "//tests:__subpackages__", ], deps = [ - ":inotify_watcher_interface", - "@googletest//:gtest", - ], -) - -cc_library( - name = "inotify_iterator", - hdrs = [ - "inotify_iterator.hpp", - ], - srcs = [ - "details/posix/inotify_iterator.cpp" - ], - include_prefix = "score/mw/launch_manager/osal", - strip_include_prefix = "/score/launch_manager/src/daemon/src/osal", - visibility = [ - "//score:__subpackages__", - "//tests:__subpackages__", - ], - deps = [ - ":inotify_watcher_interface", "//score/launch_manager/src/daemon/src/common:log", "@score_baselibs//score/result", - "//score/launch_manager:error", + "@score_baselibs//score/language/futurecpp", "@score_baselibs//score/os/utils/inotify:inotify_instance", - "@score_baselibs//score/os/utils/inotify:inotify_instance_impl", "@score_baselibs//score/os/utils/inotify:inotify_event", + "@score_baselibs//score/os/utils/inotify:inotify_instance_impl", "@score_baselibs//score/os/utils/inotify:inotify_watch_descriptor", "@score_baselibs//score/language/safecpp/string_view:zstring_view", ], ) cc_test( - name = "inotify_iterator_UT", - srcs = ["details/posix/inotify_iterator_UT.cpp"], - visibility = ["//tests:__subpackages__"], + name = "inotify_range_UT", + srcs = [ + "inotify_range_UT.cpp" + ], deps = [ - "@score_baselibs//score/os/utils/inotify:inotify_instance_mock", - ":inotify_iterator", + ":inotify_range", "@googletest//:gtest_main", - ], + "@score_baselibs//score/os/utils/inotify:inotify_instance_mock", + ] ) cc_library( diff --git a/score/launch_manager/src/daemon/src/osal/details/posix/inotify_iterator.cpp b/score/launch_manager/src/daemon/src/osal/details/posix/inotify_iterator.cpp deleted file mode 100644 index fe3cd11eb..000000000 --- a/score/launch_manager/src/daemon/src/osal/details/posix/inotify_iterator.cpp +++ /dev/null @@ -1,155 +0,0 @@ -#include "score/mw/launch_manager/osal/inotify_iterator.hpp" -#include "score/launch_manager/src/daemon/src/common/log.hpp" -#include "score/launch_manager/src/execution_error.h" -#include "score/os/utils/inotify/inotify_instance_impl.h" -#include -#include - -#include -#include -#include - -INotifyWatcher::INotifyWatcher(std::unique_ptr instance) noexcept - : instance_(std::move(instance)) -{ -} - -score::Result INotifyWatcher::Create(std::unique_ptr inotify_instance) noexcept -{ - return INotifyWatcher{std::move(inotify_instance)}; -} - -score::Result INotifyWatcher::Create() noexcept -{ - auto instance = std::make_unique(); - auto is_valid = instance->IsValid(); - if (!is_valid.has_value()) - { - return score::MakeUnexpected(score::mw::lifecycle::ExecErrc::kCommunicationError); - } - return INotifyWatcher{std::move(instance)}; -} - - -int INotifyWatcher::add_watch(std::string_view path, uint32_t mask) const noexcept -{ - if (!instance_) - { - return -1; - } - - // Convert path to zstring_view (assuming path is null-terminated) - score::safecpp::zstring_view zpath(path.data(), path.size()); - - // Convert mask to EventMask - auto event_mask = static_cast(mask); - - auto result = instance_->AddWatch(zpath, event_mask); - if (!result.has_value()) - { - LM_LOG_ERROR() << "Couldn't add watch"; - return -1; - } - - last_watch_descriptor_ = result.value(); - return last_watch_descriptor_.GetUnderlying(); -} - -void INotifyWatcher::interrupt() const noexcept -{ - if (instance_) - { - instance_->Close(); - } -} - -INotifyWatcher::iterator INotifyWatcher::begin() -{ - SCORE_LANGUAGE_FUTURECPP_ASSERT_MESSAGE(instance_ != nullptr, "INotify instance not initialized!"); - - iterator out{this}; - ++out; - return out; -} - -INotifyWatcher::iterator INotifyWatcher::end() -{ - return iterator{nullptr}; -} - -INotifyWatcher::iterator::iterator(INotifyWatcher* watch_ptr) : watcher_(watch_ptr) {} - -INotifyWatcher::iterator::reference INotifyWatcher::iterator::operator*() const -{ - return events_[event_index_]; -} - -INotifyWatcher::iterator::pointer INotifyWatcher::iterator::operator->() const -{ - return &events_[event_index_]; -} - -INotifyWatcher::iterator& INotifyWatcher::iterator::operator++() -{ - if (watcher_ == nullptr) - { - // don't advance over end() - return *this; - } - if (!advance()) - { - // become end() - watcher_ = nullptr; - } - return *this; -} - -INotifyWatcher::iterator INotifyWatcher::iterator::operator++(int) -{ - auto tmp = *this; - ++(*this); - return tmp; -} - -bool INotifyWatcher::iterator::operator==(const iterator& other) const -{ - return watcher_ == other.watcher_; -} - -bool INotifyWatcher::iterator::operator!=(const iterator& other) const -{ - return !(*this == other); -} - -bool INotifyWatcher::iterator::advance() -{ - if (!watcher_ || !watcher_->instance_) - { - return false; - } - - // Check if we have more events in the current buffer - if (event_index_ + 1 < events_.size()) - { - ++event_index_; - return true; - } - - // Need to read new events from the instance - auto result = watcher_->instance_->Read(); - if (!result.has_value()) - { - LM_LOG_ERROR() << "Failed to read inotify events"; - return false; - } - - events_ = std::move(result.value()); - if (events_.empty()) - { - // No events or interrupted - return false; - } - - event_index_ = 0; - return true; -} diff --git a/score/launch_manager/src/daemon/src/osal/details/posix/inotify_iterator_UT.cpp b/score/launch_manager/src/daemon/src/osal/details/posix/inotify_iterator_UT.cpp deleted file mode 100644 index 557f9e96a..000000000 --- a/score/launch_manager/src/daemon/src/osal/details/posix/inotify_iterator_UT.cpp +++ /dev/null @@ -1,265 +0,0 @@ -/******************************************************************************** - * 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/osal/inotify_iterator.hpp" -#include "score/os/utils/inotify/inotify_instance.h" -#include "score/os/utils/inotify/inotify_instance_mock.h" -#include -#include -#include -#include -#include -#include -#include -#include - -using score::os::Error; -using ::testing::_; -using ::testing::Return; - -using ReadRetT = - score::cpp::expected, - Error>; - -using namespace std::string_view_literals; - -class INotifyTest : public ::testing::Test -{ - protected: - constexpr static auto TEST_FILE = "/tmp/test"sv; - constexpr static auto TEST_DIR = "/tmp"sv; - score::os::InotifyInstanceMock inotify_mock{}; - - void SetUp() override - { - RecordProperty("TestType", "interface-test"); - RecordProperty("DerivationTechnique", "explorative-testing "); - - ON_CALL(inotify_mock, AddWatch(_, _)).WillByDefault(Return(score::os::InotifyWatchDescriptor{0})); - } - - void TearDown() override - { - ::unlink(TEST_FILE.data()); - } - - static ::testing::AssertionResult touch_file(std::string_view path = TEST_FILE) - { - auto fd = ::open(path.begin(), O_CREAT | O_WRONLY, 0644); - if (fd < 0) - { - return ::testing::AssertionFailure() << "Failed to open file " << std::strerror(errno); - } - - ::close(fd); - return ::testing::AssertionSuccess(); - } - - /// @brief Helper to make events - static score::os::InotifyEvent MakeEvent(const std::string_view name, int wd = 1, int cookie = 1, int mask = 1) - { - // len should include terminating '\0' for inotify-style names - auto len = static_cast(name.size() + 1); - - auto* event = static_cast(std::malloc(sizeof(inotify_event) + len)); - - event->wd = wd; - event->cookie = cookie; - event->mask = mask; - event->len = len; - std::memcpy(event->name, name.begin(), len); // copies '\0' too - - auto output = score::os::InotifyEvent(*event); - // os event copies the data se we can free - std::free(event); - - return output; - } -}; - -TEST_F(INotifyTest, INotify_Init) -{ - - ReadRetT out {}; - out->push_back(MakeEvent("hello")); - - ReadRetT empty {}; - - auto mock_ptr = std::make_unique(); - EXPECT_CALL(*mock_ptr, AddWatch(_, _)).WillOnce(Return(score::os::InotifyWatchDescriptor{1})); - EXPECT_CALL(*mock_ptr, Read()).WillOnce(Return(out)).WillOnce(Return(empty)); - EXPECT_CALL(*mock_ptr, Close()); - - /// Create the Watcher - auto res = INotifyWatcher::Create(std::move(mock_ptr)); - ASSERT_TRUE(res.has_value()); - auto watcher = std::move(res).value(); - auto watch = watcher.add_watch(TEST_DIR.begin(), IN_CREATE); - EXPECT_TRUE(watch > 0); - - std::uint8_t events_recieved{0}; - std::vector out_names {}; - - // Given we have a file watch in /tmp - std::thread watcher_thread([&watcher, &out_names, &events_recieved]() { - for (const INotifyEvent& event : watcher) - { - events_recieved++; - - out_names.emplace_back(event.GetName()); - if (events_recieved == 1) - { - watcher.interrupt(); - } - } - }); - - // When we touch a file in /tmp to trigger the event... - ASSERT_TRUE(touch_file(TEST_FILE)); - - - // ...Then one event shall be seen - watcher_thread.join(); - EXPECT_STREQ(out_names[0].c_str(), "hello"); -} - -TEST_F(INotifyTest, IteratorArrowOperator) -{ - ReadRetT out {}; - out->push_back(MakeEvent("test_file", 1, 1, IN_CREATE)); - ReadRetT empty {}; - - auto mock_ptr = std::make_unique(); - EXPECT_CALL(*mock_ptr, AddWatch(_, _)).WillOnce(Return(score::os::InotifyWatchDescriptor{1})); - EXPECT_CALL(*mock_ptr, Read()).WillOnce(Return(out)); - - auto res = INotifyWatcher::Create(std::move(mock_ptr)); - ASSERT_TRUE(res.has_value()); - auto watcher = std::move(res).value(); - ASSERT_EQ(watcher.add_watch(TEST_DIR.begin(), IN_CREATE), 1); - - auto it = watcher.begin(); - EXPECT_EQ(it->GetCookie(), 1); -} - -TEST_F(INotifyTest, PostIncrementOperator) -{ - ReadRetT out {}; - out->push_back(MakeEvent("test")); - ReadRetT empty {}; - - auto mock_ptr = std::make_unique(); - EXPECT_CALL(*mock_ptr, AddWatch(_, _)).WillOnce(Return(score::os::InotifyWatchDescriptor{1})); - EXPECT_CALL(*mock_ptr, Read()).WillOnce(Return(out)).WillOnce(Return(empty)); - - auto res = INotifyWatcher::Create(std::move(mock_ptr)); - ASSERT_TRUE(res.has_value()); - auto watcher = std::move(res).value(); - ASSERT_EQ(watcher.add_watch(TEST_DIR.begin(), IN_CREATE), 1); - - auto it = watcher.begin(); - auto old_it = it++; - EXPECT_NE(old_it, it); -} - -TEST_F(INotifyTest, MultipleEventsInBuffer) -{ - ReadRetT out {}; - out->push_back(MakeEvent("file1", 1, 1, IN_CREATE)); - out->push_back(MakeEvent("file2", 2, 2, IN_MODIFY)); - ReadRetT empty {}; - - auto mock_ptr = std::make_unique(); - EXPECT_CALL(*mock_ptr, AddWatch(_, _)).WillOnce(Return(score::os::InotifyWatchDescriptor{1})); - EXPECT_CALL(*mock_ptr, Read()).WillOnce(Return(out)).WillOnce(Return(empty)); - - auto res = INotifyWatcher::Create(std::move(mock_ptr)); - ASSERT_TRUE(res.has_value()); - auto watcher = std::move(res).value(); - ASSERT_EQ(watcher.add_watch(TEST_DIR.begin(), IN_CREATE), 1); - - std::uint8_t event_count = 0; - for (const auto& event : watcher) - { - static_cast(event); - event_count++; - } - - EXPECT_EQ(event_count, 2); -} - -TEST_F(INotifyTest, IncrementEndIterator) -{ - auto mock_ptr = std::make_unique(); - - auto res = INotifyWatcher::Create(std::move(mock_ptr)); - ASSERT_TRUE(res.has_value()); - auto watcher = std::move(res).value(); - - auto it = watcher.end(); - ++it; - EXPECT_EQ(it, watcher.end()); -} - -TEST_F(INotifyTest, CreateWithoutParameters) -{ - auto res = INotifyWatcher::Create(); - EXPECT_TRUE(res.has_value()); -} - -TEST_F(INotifyTest, AddWatchFailure) -{ - auto mock_ptr = std::make_unique(); - EXPECT_CALL(*mock_ptr, AddWatch(_, _)) - .WillOnce(Return(score::cpp::unexpected(score::os::Error::createFromErrno(EINVAL)))); - - auto res = INotifyWatcher::Create(std::move(mock_ptr)); - ASSERT_TRUE(res.has_value()); - auto watcher = std::move(res).value(); - - auto watch = watcher.add_watch(TEST_DIR.begin(), IN_CREATE); - EXPECT_EQ(watch, -1); -} - -TEST_F(INotifyTest, ReadFailure) -{ - auto mock_ptr = std::make_unique(); - EXPECT_CALL(*mock_ptr, AddWatch(_, _)).WillOnce(Return(score::os::InotifyWatchDescriptor{1})); - EXPECT_CALL(*mock_ptr, Read()) - .WillOnce(Return(score::cpp::unexpected(score::os::Error::createFromErrno(EINVAL)))); - - auto res = INotifyWatcher::Create(std::move(mock_ptr)); - ASSERT_TRUE(res.has_value()); - auto watcher = std::move(res).value(); - auto watch = watcher.add_watch(TEST_DIR.begin(), IN_CREATE); - EXPECT_GT(watch, 0); - - auto it = watcher.begin(); - EXPECT_EQ(it, watcher.end()); -} - -TEST_F(INotifyTest, AddWatchAfterMove) -{ - auto mock_ptr = std::make_unique(); - - auto res = INotifyWatcher::Create(std::move(mock_ptr)); - ASSERT_TRUE(res.has_value()); - auto watcher = std::move(res).value(); - - // Move the watcher, leaving the original in a moved-from state - auto moved_watcher = std::move(watcher); - - // Try to add_watch on the moved-from watcher (instance_ should be null) - auto watch = watcher.add_watch(TEST_DIR.begin(), IN_CREATE); - EXPECT_EQ(watch, -1); -} diff --git a/score/launch_manager/src/daemon/src/osal/inotify_iterator.hpp b/score/launch_manager/src/daemon/src/osal/inotify_iterator.hpp deleted file mode 100644 index ae26cbc71..000000000 --- a/score/launch_manager/src/daemon/src/osal/inotify_iterator.hpp +++ /dev/null @@ -1,115 +0,0 @@ -#include -#include -#include -#include -#include -#include -#include - -#include "score/result/result.h" -#include "score/os/utils/inotify/inotify_instance.h" -#include "score/os/utils/inotify/inotify_instance_impl.h" -#include "score/os/utils/inotify/inotify_event.h" -#include "score/os/utils/inotify/inotify_watch_descriptor.h" -#include "score/mw/launch_manager/osal/inotify_watcher_interface.hpp" - -// Use the baselibs inotify event type -using INotifyEvent = score::os::InotifyEvent; - -class INotifyWatcher : public INotifyWatcherInterface -{ - private: - /// @brief Use the `Create()` method to create the `INotifyWatcher`. - explicit INotifyWatcher(std::unique_ptr instance) noexcept; - - // forward declaration for iterator class - class iterator; - - public: - /// @brief Creates a INotifyWatcher or returns an error. If nullptr is given this will create the Impl. - [[nodiscard]] static score::Result Create(std::unique_ptr inotify_instance) noexcept; - - /// @brief Creates a INotifyWatcher or returns an error. - [[nodiscard]] static score::Result Create() noexcept; - - ~INotifyWatcher() = default; - - INotifyWatcher(INotifyWatcher&& other) noexcept = default; - INotifyWatcher& operator=(INotifyWatcher&& other) noexcept = default; - - INotifyWatcher(const INotifyWatcher& other) = delete; - INotifyWatcher& operator=(const INotifyWatcher& other) = delete; - - /// @brief Adds the given file to that watch with the given event mask. - [[nodiscard]] int add_watch(std::string_view path, uint32_t mask) const noexcept override; - - /// @brief Interupt the reading of the events. - void interrupt() const noexcept override; - - /// @brief Gives an iterator to read new events from the registered - /// watches. - [[nodiscard]] iterator begin(); - - /// @brief Returns the sentinal iterator. - /// @details If iterating over the iterator the `interrupt()` has to be - /// called. - [[nodiscard]] iterator end(); - - private: - /// @brief The underlying inotify instance from baselibs - std::unique_ptr instance_; - - /// @brief Watch descriptor stored for the last add_watch call - mutable score::os::InotifyWatchDescriptor last_watch_descriptor_{-1}; - - /// @brief An iterator that reads the event queue - /// - /// @details - /// ``` - /// auto res = INotifyWatcher::Create(); - /// auto watcher = std::move(res).value(); - /// auto watch = watcher.add_watch("/tmp", IN_CREATE); - /// - /// for (const auto& event : watcher) - /// { - /// std::cout << event.GetWatchDescriptor().GetUnderlying() << std::endl; - /// std::cout << event.GetName() << std::endl; - /// } - /// ``` - class iterator - { - public: - using iterator_category = std::input_iterator_tag; - using value_type = INotifyEvent; - using difference_type = std::ptrdiff_t; - using pointer = const INotifyEvent*; - using reference = const INotifyEvent&; - - explicit iterator(INotifyWatcher* watch_ptr = nullptr); - - [[nodiscard]] reference operator*() const; - - [[nodiscard]] pointer operator->() const; - - iterator& operator++(); - - iterator operator++(int); - - [[nodiscard]] bool operator==(const iterator& other) const; - - [[nodiscard]] bool operator!=(const iterator& other) const; - - private: - /// @brief Pointer to the parent watcher. - INotifyWatcher* watcher_ = nullptr; - - /// @brief Buffer of events read from the instance. - score::cpp::static_vector events_{}; - - /// @brief Current position in the events buffer. - size_t event_index_{0}; - - /// @brief Returns false if interrupted or error. - [[nodiscard]] bool advance(); - }; -}; diff --git a/score/launch_manager/src/daemon/src/osal/inotify_range.hpp b/score/launch_manager/src/daemon/src/osal/inotify_range.hpp new file mode 100644 index 000000000..3e8487f36 --- /dev/null +++ b/score/launch_manager/src/daemon/src/osal/inotify_range.hpp @@ -0,0 +1,150 @@ +#ifndef INOTIFY_RANGE_HPP_ +#define INOTIFY_RANGE_HPP_ + +#include +#include +#include +#include + +#include "score/assert.hpp" +#include "score/os/utils/inotify/inotify_event.h" +#include "score/os/utils/inotify/inotify_instance.h" + +namespace score::mw::lifecycle +{ + +/// @brief A wrapper around score::os::InotiyInstance to get iterator syntax. +class INotifyRange +{ + public: + explicit INotifyRange(std::unique_ptr instance) noexcept + : instance_(std::move(instance)) + { + } + + ~INotifyRange() = default; + + INotifyRange(INotifyRange&& other) noexcept = default; + INotifyRange& operator=(INotifyRange&& other) noexcept = default; + + INotifyRange(const INotifyRange& other) = delete; + INotifyRange& operator=(const INotifyRange& other) = delete; + + /// @brief The iterator giving InotifyEvents. + /// @warning The iterator's lifetime must not exceed the `INotifyRange` it + /// was obtained from. + class iterator + { + public: + using iterator_category = std::input_iterator_tag; + using value_type = score::os::InotifyEvent; + using difference_type = std::ptrdiff_t; + using pointer = const score::os::InotifyEvent*; + using reference = const score::os::InotifyEvent&; + + explicit iterator(score::os::InotifyInstance* instance = nullptr) noexcept : instance_(instance) + { + if ((instance_ != nullptr) && !advance()) + { + instance_ = nullptr; + } + } + + [[nodiscard]] reference operator*() const + { + return events_[event_index_]; + } + + [[nodiscard]] pointer operator->() const + { + return &events_[event_index_]; + } + + iterator& operator++() + { + if (instance_ == nullptr) + { + return *this; + } + + if (!advance()) + { + instance_ = nullptr; + } + + return *this; + } + + iterator operator++(int) + { + auto tmp = *this; + ++(*this); + return tmp; + } + + [[nodiscard]] bool operator==(const iterator& other) const + { + return instance_ == other.instance_; + } + + [[nodiscard]] bool operator!=(const iterator& other) const + { + return !(*this == other); + } + + private: + [[nodiscard]] bool advance() + { + + SCORE_LANGUAGE_FUTURECPP_ASSERT_MESSAGE( + instance_ != nullptr, "The parent INotifyRange must have been deleted, documented as UB!"); + + const bool events_in_internal_buffer = event_index_ + 1U < events_.size(); + if (events_in_internal_buffer) + { + ++event_index_; + return true; + } + + auto result = instance_->Read(); + if (!result.has_value()) + { + return false; + } + + events_ = std::move(result.value()); + + if (events_.empty()) + { + return false; + } + + event_index_ = 0U; + return true; + } + + public: + score::os::InotifyInstance* instance_{nullptr}; + + score::cpp::static_vector events_{}; + + std::size_t event_index_{0U}; + }; + + [[nodiscard]] iterator begin() noexcept + { + return iterator{instance_.get()}; + } + + [[nodiscard]] iterator end() noexcept + { + return iterator{nullptr}; + } + + private: + std::unique_ptr instance_; +}; + +} // namespace score::mw::lifecycle + +#endif // INOTIFY_RANGE_HPP_ diff --git a/score/launch_manager/src/daemon/src/osal/inotify_range_UT.cpp b/score/launch_manager/src/daemon/src/osal/inotify_range_UT.cpp new file mode 100644 index 000000000..f2449b7a1 --- /dev/null +++ b/score/launch_manager/src/daemon/src/osal/inotify_range_UT.cpp @@ -0,0 +1,116 @@ +/******************************************************************************** + * 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 "score/mw/launch_manager/osal/inotify_range.hpp" +#include "score/os/utils/inotify/inotify_instance_mock.h" +#include +#include + +namespace score::mw::lifecycle::testing +{ + +using score::os::Error; +using score::os::MakeFakeEvent; +using ::testing::_; +using ::testing::Return; + +using ReadRetT = + score::cpp::expected, + Error>; + +using namespace std::string_view_literals; + +class INotifyTest : public ::testing::Test +{ + protected: + constexpr static auto TEST_FILE = "/tmp/test"sv; + constexpr static auto TEST_DIR = "/tmp"sv; + + void SetUp() override + { + RecordProperty("TestType", "interface-test"); + RecordProperty("DerivationTechnique", "explorative-testing "); + } +}; + +TEST(INotifyRange, NormalUsage) +{ + + /// Given we Read() one event + ReadRetT out{}; + out->push_back(MakeFakeEvent(1,1,1, "hello")); + ReadRetT empty{}; + auto mock_ptr = std::make_unique(); + EXPECT_CALL(*mock_ptr, Read()).WillOnce(Return(out)).WillOnce(Return(empty)); + + INotifyRange range{std::move(mock_ptr)}; + + /// Expect the first value will be given. + auto iterator = range.begin(); + auto event = *iterator; + EXPECT_EQ(event.GetCookie(), 1); + + /// Expect incrementing will give you an end iterator. + iterator++; + EXPECT_EQ(iterator, range.end()); +} + +TEST(INotifyRange, AdvanceWithinBufferedEvents) +{ + /// Given we Read() 2 events, then 0 events + ReadRetT out{}; + out->push_back(MakeFakeEvent(1, 1, 10, "first")); + out->push_back(MakeFakeEvent(2, 1, 20, "second")); + ReadRetT empty{}; + auto mock_ptr = std::make_unique(); + EXPECT_CALL(*mock_ptr, Read()).WillOnce(Return(out)).WillOnce(Return(empty)); + + INotifyRange range{std::move(mock_ptr)}; + + /// Expect that the first value will equal to the first event. + auto iterator = range.begin(); + EXPECT_EQ(iterator->GetCookie(), 10); + + /// Expect that the second value will equal to the second event. + ++iterator; + EXPECT_EQ(iterator->GetCookie(), 20); + + /// Expect that incrementing will give you an end iterator. + ++iterator; + EXPECT_EQ(iterator, range.end()); +} + +TEST(INotifyRange, AdvanceEndIterator) +{ + auto mock_ptr = std::make_unique(); + INotifyRange range{std::move(mock_ptr)}; + + /// Expect that incrementing end iterator still equals to an end iterator. + auto iterator = range.end(); + iterator++; + EXPECT_EQ(iterator, range.end()); +} + +TEST(INotifyRange, AdvanceError) +{ + + /// Given we get an Error from the Read(). + auto mock_ptr = std::make_unique(); + EXPECT_CALL(*mock_ptr, Read()).WillOnce(Return(score::cpp::unexpected(score::os::Error::createFromErrno(EINVAL)))); + + INotifyRange range{std::move(mock_ptr)}; + + /// Expect that the given iterator is empty. + EXPECT_EQ(range.begin(), range.end()); +} + +} // namespace score::mw::lifecycle::testing diff --git a/score/launch_manager/src/daemon/src/osal/inotify_watcher_interface.hpp b/score/launch_manager/src/daemon/src/osal/inotify_watcher_interface.hpp deleted file mode 100644 index 7ae0a90e2..000000000 --- a/score/launch_manager/src/daemon/src/osal/inotify_watcher_interface.hpp +++ /dev/null @@ -1,31 +0,0 @@ -/******************************************************************************** - * 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 - ********************************************************************************/ - -#pragma once - -#include -#include - -/// @brief Interface for file system notification watchers -class INotifyWatcherInterface -{ - public: - virtual ~INotifyWatcherInterface() = default; - - /// @brief Adds the given file to that watch with the given event mask. - /// @return Watch descriptor on success, -1 on failure - [[nodiscard]] virtual int add_watch(std::string_view path, uint32_t mask) const noexcept = 0; - - /// @brief Interrupt the reading of events. - virtual void interrupt() const noexcept = 0; -}; diff --git a/score/launch_manager/src/daemon/src/osal/inotify_watcher_mock.hpp b/score/launch_manager/src/daemon/src/osal/inotify_watcher_mock.hpp deleted file mode 100644 index 9e33003b5..000000000 --- a/score/launch_manager/src/daemon/src/osal/inotify_watcher_mock.hpp +++ /dev/null @@ -1,27 +0,0 @@ -/******************************************************************************** - * 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 - ********************************************************************************/ - -#pragma once - -#include "inotify_watcher_interface.hpp" -#include -#include -#include - -/// @brief Mock implementation of INotifyWatcherInterface for testing -class INotifyWatcherMock : public INotifyWatcherInterface -{ - public: - MOCK_METHOD(int, add_watch, (std::string_view path, uint32_t mask), (const, noexcept, override)); - MOCK_METHOD(void, interrupt, (), (const, noexcept, override)); -}; 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 23a194897..265c792d4 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 @@ -142,7 +142,7 @@ cc_library( }), 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__"], + visibility = ["//score/launch_manager/src/daemon/src/process_group_manager:__pkg__"], deps = [ ":icomponent", ":safe_process_map", @@ -177,7 +177,7 @@ cc_library( hdrs = ["graph.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__"], + visibility = ["//score/launch_manager/src/daemon/src/process_group_manager:__pkg__"], deps = [ ":component_event_queue", ":component_task", @@ -197,7 +197,7 @@ cc_library( hdrs = ["safe_process_map.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__"], + visibility = ["//score/launch_manager/src/daemon/src/process_group_manager:__pkg__"], deps = [ ":icomponent_controller", "//score/launch_manager/src/daemon/src/process_group_manager:iprocess", @@ -221,7 +221,7 @@ cc_library( hdrs = ["os_handler.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__"], + visibility = ["//score/launch_manager/src/daemon/src/process_group_manager:__pkg__"], deps = [ ":safe_process_map", "//score/launch_manager/src/daemon/src/common:log", @@ -274,7 +274,7 @@ cc_test( cc_library( name = "process_launcher", srcs = ["process_launcher.cpp"], - visibility = ["//score/launch_manager/src/daemon/src/process_group_manager:__subpackages__"], + visibility = ["//score/launch_manager/src/daemon/src/process_group_manager:__pkg__"], deps = [ "//score/launch_manager/src/daemon/src/common:log", "//score/launch_manager/src/daemon/src/common:signal_safe_log", @@ -302,7 +302,7 @@ cc_library( "//config:lm_use_new_configuration": ["USE_NEW_CONFIGURATION"], "//conditions:default": [], }), - visibility = ["//score/launch_manager/src/daemon/src/process_group_manager:__subpackages__"], + visibility = ["//score/launch_manager/src/daemon/src/process_group_manager:__pkg__"], deps = [ ":component_event_queue", ":graph", From 371082b722f9f4a4adf82cedfad05fb6951cd995 Mon Sep 17 00:00:00 2001 From: Maciej Kaszynski Date: Fri, 31 Jul 2026 16:06:00 +0100 Subject: [PATCH 7/7] Some fixes --- .../launch_manager/src/daemon/src/osal/BUILD | 11 +++++----- .../src/daemon/src/osal/inotify_range.hpp | 18 +++++++++++++++++ .../src/daemon/src/osal/inotify_range_UT.cpp | 20 +++++++------------ 3 files changed, 30 insertions(+), 19 deletions(-) diff --git a/score/launch_manager/src/daemon/src/osal/BUILD b/score/launch_manager/src/daemon/src/osal/BUILD index c3269be5b..466087758 100644 --- a/score/launch_manager/src/daemon/src/osal/BUILD +++ b/score/launch_manager/src/daemon/src/osal/BUILD @@ -109,7 +109,6 @@ cc_library( ], ) - cc_library( name = "inotify_range", hdrs = [ @@ -123,26 +122,26 @@ cc_library( ], deps = [ "//score/launch_manager/src/daemon/src/common:log", - "@score_baselibs//score/result", "@score_baselibs//score/language/futurecpp", - "@score_baselibs//score/os/utils/inotify:inotify_instance", + "@score_baselibs//score/language/safecpp/string_view:zstring_view", "@score_baselibs//score/os/utils/inotify:inotify_event", + "@score_baselibs//score/os/utils/inotify:inotify_instance", "@score_baselibs//score/os/utils/inotify:inotify_instance_impl", "@score_baselibs//score/os/utils/inotify:inotify_watch_descriptor", - "@score_baselibs//score/language/safecpp/string_view:zstring_view", + "@score_baselibs//score/result", ], ) cc_test( name = "inotify_range_UT", srcs = [ - "inotify_range_UT.cpp" + "inotify_range_UT.cpp", ], deps = [ ":inotify_range", "@googletest//:gtest_main", "@score_baselibs//score/os/utils/inotify:inotify_instance_mock", - ] + ], ) cc_library( diff --git a/score/launch_manager/src/daemon/src/osal/inotify_range.hpp b/score/launch_manager/src/daemon/src/osal/inotify_range.hpp index 3e8487f36..e30cae8b5 100644 --- a/score/launch_manager/src/daemon/src/osal/inotify_range.hpp +++ b/score/launch_manager/src/daemon/src/osal/inotify_range.hpp @@ -1,3 +1,16 @@ +/******************************************************************************** + * 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 INOTIFY_RANGE_HPP_ #define INOTIFY_RANGE_HPP_ @@ -62,11 +75,13 @@ class INotifyRange iterator& operator++() { + // we are end iterator if (instance_ == nullptr) { return *this; } + // become end iterator if (!advance()) { instance_ = nullptr; @@ -124,10 +139,13 @@ class INotifyRange } public: + /// @brief The underlying `InotifyInstance` instance. score::os::InotifyInstance* instance_{nullptr}; + /// @brief Internal buffer of events. score::cpp::static_vector events_{}; + /// @brief Index to the next event to give from the internal buffer. std::size_t event_index_{0U}; }; diff --git a/score/launch_manager/src/daemon/src/osal/inotify_range_UT.cpp b/score/launch_manager/src/daemon/src/osal/inotify_range_UT.cpp index f2449b7a1..251526bba 100644 --- a/score/launch_manager/src/daemon/src/osal/inotify_range_UT.cpp +++ b/score/launch_manager/src/daemon/src/osal/inotify_range_UT.cpp @@ -1,5 +1,5 @@ /******************************************************************************** - * Copyright (c) 2025 Contributors to the Eclipse Foundation + * Copyright (c) 2026 Contributors to the Eclipse Foundation * * See the NOTICE file(s) distributed with this work for additional * information regarding copyright ownership. @@ -20,7 +20,6 @@ namespace score::mw::lifecycle::testing using score::os::Error; using score::os::MakeFakeEvent; -using ::testing::_; using ::testing::Return; using ReadRetT = @@ -29,22 +28,18 @@ using ReadRetT = using namespace std::string_view_literals; -class INotifyTest : public ::testing::Test +class INotifyRangeUT : public ::testing::Test { protected: - constexpr static auto TEST_FILE = "/tmp/test"sv; - constexpr static auto TEST_DIR = "/tmp"sv; - void SetUp() override { RecordProperty("TestType", "interface-test"); - RecordProperty("DerivationTechnique", "explorative-testing "); + RecordProperty("DerivationTechnique", "equivalence-classes"); } }; -TEST(INotifyRange, NormalUsage) +TEST_F(INotifyRangeUT, NormalUsage) { - /// Given we Read() one event ReadRetT out{}; out->push_back(MakeFakeEvent(1,1,1, "hello")); @@ -64,7 +59,7 @@ TEST(INotifyRange, NormalUsage) EXPECT_EQ(iterator, range.end()); } -TEST(INotifyRange, AdvanceWithinBufferedEvents) +TEST_F(INotifyRangeUT, AdvanceWithinBufferedEvents) { /// Given we Read() 2 events, then 0 events ReadRetT out{}; @@ -89,7 +84,7 @@ TEST(INotifyRange, AdvanceWithinBufferedEvents) EXPECT_EQ(iterator, range.end()); } -TEST(INotifyRange, AdvanceEndIterator) +TEST_F(INotifyRangeUT, AdvanceEndIterator) { auto mock_ptr = std::make_unique(); INotifyRange range{std::move(mock_ptr)}; @@ -100,9 +95,8 @@ TEST(INotifyRange, AdvanceEndIterator) EXPECT_EQ(iterator, range.end()); } -TEST(INotifyRange, AdvanceError) +TEST_F(INotifyRangeUT, AdvanceError) { - /// Given we get an Error from the Read(). auto mock_ptr = std::make_unique(); EXPECT_CALL(*mock_ptr, Read()).WillOnce(Return(score::cpp::unexpected(score::os::Error::createFromErrno(EINVAL))));