From c0eeccb18a89a0189a4c71ab41b324cf47839956 Mon Sep 17 00:00:00 2001 From: Manish Tiwari Date: Mon, 10 Aug 2026 05:07:09 -0500 Subject: [PATCH 1/2] inotify: stop active watchers when sync is disabled When sync is disabled, active DataWatcher instances may remain blocked waiting for an inotify event in onDataChange(). This prevents the monitor coroutines from exiting immediately. If sync is re-enabled before the old monitor coroutine exits, the code can still use the existing watcher. The new monitor task can enter onDataChange() while the old monitor task is already waiting on the same fdio object. sdbusplus fdio rejects a second pending await on the same fdio object, so the service terminates with: ``` what(): fdio_completion started with another await already pending! ``` Add DataWatcher::stop() to remove the active inotify watch descriptors. Removing a watch generates an IN_IGNORED event, which makes the inotify file descriptor readable and wakes the pending fdio::next() operation. IN_IGNORED is not treated as a data change, so no sync operation is triggered. Once onDataChange() returns, the monitor loop observes that sync is disabled and exits. The existing scope cleanup then removes the DataWatcher from the active watcher map. During failover, the passive BMC becomes active without restarting the application. Any watcher created for the previous BMC role may therefore remain active. Stopping the existing watchers ensures that they are removed before sync events are started again for the new BMC role. Call stop() for all active watchers when the DisableSync property is set. Change-Id: I7e21ded548ff56eeaf7885aeafdefc7fd3a3c13c Signed-off-by: Manish Tiwari --- src/data_watcher.cpp | 11 +++++++++++ src/data_watcher.hpp | 10 ++++++++++ src/manager.cpp | 18 +++++++++--------- src/manager.hpp | 8 ++++++++ 4 files changed, 38 insertions(+), 9 deletions(-) diff --git a/src/data_watcher.cpp b/src/data_watcher.cpp index 790044e..ed9ffb7 100644 --- a/src/data_watcher.cpp +++ b/src/data_watcher.cpp @@ -56,6 +56,17 @@ int DataWatcher::inotifyInit() const return fd; } +void DataWatcher::stop() +{ + lg2::debug("Stopping DataWatcher for [{PATH}], removing [{COUNT}] watches", + "PATH", _dataPathToWatch, "COUNT", _watchDescriptors.size()); + + auto wds = _watchDescriptors | std::views::keys | + std::ranges::to(); + + std::ranges::for_each(wds, [this](int wd) { removeWatch(wd); }); +} + std::string DataWatcher::eventName(uint32_t eventMask) { std::vector events{}; diff --git a/src/data_watcher.hpp b/src/data_watcher.hpp index 8fe2ce5..ade8bed 100644 --- a/src/data_watcher.hpp +++ b/src/data_watcher.hpp @@ -12,6 +12,7 @@ #include #include #include +#include namespace data_sync::watch::inotify { @@ -105,6 +106,15 @@ class DataWatcher */ sdbusplus::async::task onDataChange(); + /** + * @brief Stop all active inotify watches. + * + * Removes all watch descriptors, causing the kernel to emit + * IN_IGNORED which unblocks any pending co_await on onDataChange(). + * The inotify fd is closed by the destructor after the coroutine exits. + */ + void stop(); + /** * @brief Get the current watch descriptors map * diff --git a/src/manager.cpp b/src/manager.cpp index 606745a..8082000 100644 --- a/src/manager.cpp +++ b/src/manager.cpp @@ -293,14 +293,6 @@ sdbusplus::async::task<> Manager::startSyncEvents() } else if (dataSyncCfg._syncType == Deferred) { - if ((dataSyncCfg._syncDirection == Bidirectional) && - _activeWatchers.contains(dataSyncCfg._path)) - { - lg2::debug( - "Bidirectional watcher already exists for {PATH}, skipping duplicate watcher", - "PATH", dataSyncCfg._path); - return; - } try { this->_ctx.spawn(this->monitorDeferredDataToSync(dataSyncCfg)); @@ -331,6 +323,14 @@ sdbusplus::async::task<> Manager::startSyncEvents() co_return; } +void Manager::stopSyncEvents() +{ + for (auto& [path, watcher] : _activeWatchers) + { + watcher->stop(); + } +} + bool Manager::isRetryEligible(uint8_t errCode) noexcept { switch (errCode) @@ -966,8 +966,8 @@ void Manager::disableSyncPropChanged(bool disableSync) { if (disableSync) { - // TODO: Disable all sync events using Sender Receiver. lg2::info("Sync is Disabled, Stopping events"); + stopSyncEvents(); } else { diff --git a/src/manager.hpp b/src/manager.hpp index 87ee5fb..4a614d5 100644 --- a/src/manager.hpp +++ b/src/manager.hpp @@ -188,6 +188,14 @@ class Manager */ sdbusplus::async::task<> startSyncEvents(); + /** + * @brief Stop all active data change watchers. + * + * Stops each watcher in _activeWatchers, allowing their monitor + * coroutines to exit cleanly. + */ + void stopSyncEvents(); + /** * @brief API responsible to trigger sibling notification if required. * From 895434ca8b57d2815604456cb1389b86db8b8e9a Mon Sep 17 00:00:00 2001 From: Manish Tiwari Date: Mon, 17 Aug 2026 02:15:14 -0500 Subject: [PATCH 2/2] Revert "manager: avoid duplicate monitor for bidirectional watchers (#110)" This fix is no longer needed as the root cause is addressed in the commit c0eeccb. This reverts commit 79c88795fe6809e09071538f6c307f8843d9aecf. Change-Id: I79a55c8dcecac6f1ed05c674546953077a45861b Signed-off-by: Manish Tiwari --- src/manager.cpp | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/manager.cpp b/src/manager.cpp index 8082000..9bf9e79 100644 --- a/src/manager.cpp +++ b/src/manager.cpp @@ -264,21 +264,9 @@ sdbusplus::async::task<> Manager::startSyncEvents() return this->isSyncEligible(dataSyncCfg); }), [this](const auto& dataSyncCfg) { - using enum config::SyncDirection; using enum config::SyncType; if (dataSyncCfg._syncType == Immediate) { - // Bidirectional watchers can already be running when sync is - // enabled again during failover. Skip creating the same watcher - // again. - if ((dataSyncCfg._syncDirection == Bidirectional) && - _activeWatchers.contains(dataSyncCfg._path)) - { - lg2::debug( - "Bidirectional watcher already exists for {PATH}, skipping duplicate watcher", - "PATH", dataSyncCfg._path); - return; - } try { this->_ctx.spawn(this->monitorDataToSync(dataSyncCfg));