RDKEMW-20646 : Using Thread::WorkerPool to send the events by karuna2git · Pull Request #322 · rdkcentral/networkmanager

karuna2git · 2026-07-06T21:09:21Z

Reason for change: All the NetworkManager Events are published from the calling thread context; somethings are from NMEvent, NMThread, somethings are from Connectivity thread and WiFiQualityMonitoring thread. When consumer is not responding properly, this will lead to extended lock & Sometimes Gnome NetworkManager Crash. To avoid this, all the events are sent thro Thread::WorkerPool that is default created by the Thunder. Test Procedure: Verify All the Networking Events
Risks: Medium
Signed-off-by: Karunakaran A <karunakaran_amirthalingam@cable.comcast.com

Copilot

Pull request overview

This PR aims to prevent NetworkManager event publishing from blocking (and potentially deadlocking/crashing) caller threads by offloading notification delivery onto Thunder’s default Core::IWorkerPool.

Changes:

Introduces a worker-pool based event dispatch path (dispatchEvent + Job) and routes existing NetworkManager notifications through it.
Updates plugin/legacy .conf.in templates to use new CMake-configurable *_AUTOSTART values and removes legacy .config templates.
Updates the unified plugin targets to build with CXX_STANDARD 17.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
plugin/NetworkManagerImplementation.h	Adds `NMPublishEvents`, `Job` dispatch wrapper, and private dispatch helpers.
plugin/NetworkManagerImplementation.cpp	Routes notification callbacks through worker pool; adds Dispatch switch-based fanout and adds LOG_ENTRY_FUNCTION() in multiple methods.
plugin/NetworkManager.config	Removed legacy plugin `.config` template.
plugin/NetworkManager.conf.in	Makes `autostart` configurable via `@PLUGIN_NETWORKMANAGER_AUTOSTART@`.
plugin/CMakeLists.txt	Adds `PLUGIN_NETWORKMANAGER_AUTOSTART` cache var; bumps unified plugin targets to C++17.
legacy/LegacyWiFiManagerAPIs.config	Removed legacy plugin `.config` template.
legacy/LegacyWiFiManagerAPIs.conf.in	Makes `autostart` configurable via `@PLUGIN_LEGACY_WIFI_AUTOSTART@`.
legacy/LegacyNetworkAPIs.config	Removed legacy plugin `.config` template.
legacy/LegacyNetworkAPIs.conf.in	Makes `autostart` configurable via `@PLUGIN_LEGACY_NW_AUTOSTART@`.
legacy/CMakeLists.txt	Adds `PLUGIN_LEGACY_*_AUTOSTART` cache vars.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

+            class EXTERNAL Job : public Core::IDispatch {
+            protected:
+                Job(NetworkManagerImplementation *pNWImpl , NMPublishEvents event, JsonObject &params)
+                    : m_jobNWImpl(pNWImpl)
+                    , _event(event)


+            {
+                JsonObject parameters;
+                parameters.FromString(jsonOfFilterScanResults);
+                dispatchEvent(NM_ON_AVAILABLESSIDS, parameters);
            }


+                case NM_ON_AVAILABLESSIDS:
+                {
+                    string jsonOfFilterScanResults;
+                    parameters.ToString(jsonOfFilterScanResults);
+                    for (const auto callback : callbacks) {
+                        callback->onAvailableSSIDs(jsonOfFilterScanResults);
+                        callback->Release();
+                    }
+                }
+                break;


+        void NetworkManagerImplementation::Dispatch(NMPublishEvents event, const JsonObject &parameters)
+        {
+            LOG_ENTRY_FUNCTION();
+            NMLOG_INFO("Posting %d Event\n", event);


Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.

+            {
+                JsonObject parameters;
+                parameters.FromString(jsonOfFilterScanResults);
+                dispatchEvent(NM_ON_AVAILABLESSIDS, parameters);
            }


+                    string jsonOfFilterScanResults;
+                    parameters.ToString(jsonOfFilterScanResults);
+                    for (const auto callback : callbacks) {
+                        callback->onAvailableSSIDs(jsonOfFilterScanResults);
+                        callback->Release();
+                    }


+            class EXTERNAL Job : public Core::IDispatch {
+            protected:
+                Job(NetworkManagerImplementation *pNWImpl , NMPublishEvents event, const JsonObject &params)
+                    : m_jobNWImpl(pNWImpl)


 set_target_properties(${MODULE_NAME} PROPERTIES
-                                     CXX_STANDARD 11
+                                     CXX_STANDARD 17
                                     CXX_STANDARD_REQUIRED YES
                                     FRAMEWORK FALSE)


 set_target_properties(${MODULE_IMPL_NAME} PROPERTIES
-                                          CXX_STANDARD 11
+                                          CXX_STANDARD 17
                                          CXX_STANDARD_REQUIRED YES
                                          FRAMEWORK FALSE)


Copilot

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.

+namespace Plugin {
+
+ENUM_CONVERSION_BEGIN(NetworkManagerImplementation::NMPublishEvents)
+    { NetworkManagerImplementation::NMPublishEvents::NM_ON_INTERFACESTATE_CHANGE, _TXT("onInterfaceStateChange") },
+    { NetworkManagerImplementation::NMPublishEvents::NM_ON_ACTIVEINTERFACE_CHANGE, _TXT("onActiveInterfaceChange") },
+    { NetworkManagerImplementation::NMPublishEvents::NM_ON_IPADDRESS_CHANGE, _TXT("onIPAddressChange") },
+    { NetworkManagerImplementation::NMPublishEvents::NM_ON_INTERNETSTATUS_CHANGE, _TXT("onInternetStatusChange") },
+    { NetworkManagerImplementation::NMPublishEvents::NM_ON_AVAILABLESSIDS, _TXT("onAvailableSSIDs") },
+    { NetworkManagerImplementation::NMPublishEvents::NM_ON_WIFISTATE_CHANGE, _TXT("onWiFiStateChange") },
+    { NetworkManagerImplementation::NMPublishEvents::NM_ON_WIFISIGNALQUALITY_CHANGE, _TXT("onWiFiSignalQualityChange") },
+ENUM_CONVERSION_END(NetworkManagerImplementation::NMPublishEvents)
+
+}


+        void NetworkManagerImplementation::enqueueEvent(NMPublishEvents event, EventDataVariant&& data)
+        {
+            LOG_ENTRY_FUNCTION();
+            {
+                std::lock_guard<std::mutex> lock(m_eventMutex);
+                m_eventQueue.push({event, std::move(data)});
+                NMLOG_DEBUG("Event %d queued, queue size: %zu", event, m_eventQueue.size());
+            }
+            m_eventCondVar.notify_one();
+        }


+        void NetworkManagerImplementation::dispatchEvent(NMPublishEvents event, const EventDataVariant& data)
+        {
+            LOG_ENTRY_FUNCTION();
+            string eventName = Core::EnumerateType<NMPublishEvents>(event).Data();
+            NMLOG_INFO("Posting %s Event", eventName.c_str());
+            std::list<Exchange::INetworkManager::INotification*> callbacks;


            m_processMonThread = std::thread(&NetworkManagerImplementation::processMonitor, this, NM_PROCESS_MONITOR_INTERVAL_SEC);
+
+            /* Start dedicated event dispatch thread */
+            m_eventThreadStop.store(false);
+            m_eventThread = std::thread(&NetworkManagerImplementation::eventThreadFunction, this);
+            NMLOG_INFO("Event dispatch thread started");


Reason for change: All the NetworkManager Events are published from the calling thread context; somethings are from NMEvent, NMThread, somethings are from Connectivity thread and WiFiQualityMonitoring thread. When consumer is not responding properly, this will lead to extended lock & Sometimes Gnome NetworkManager Crash. To avoid this, all the events are sent thro dedicated eventing thread. NOTE: The Thread::WorkerPool that is default created by the Thunder is always using the main thread when available even though worker pool is available. So this is of no use. Test Procedure: Verify All the Networking Events Risks: Medium Signed-off-by: Karunakaran A <karunakaran_amirthalingam@cable.comcast.com

Copilot

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.

+            // Log if events were discarded during shutdown
+            if (!m_eventQueue.empty()) {
+                NMLOG_WARNING("Event thread shutting down with %zu events discarded", m_eventQueue.size());
+            }


+            /* Start dedicated event dispatch thread */
+            m_eventThreadStop.store(false);
+            m_eventThread = std::thread(&NetworkManagerImplementation::eventThreadFunction, this);
+            NMLOG_INFO("Event dispatch thread started");


+        void NetworkManagerImplementation::enqueueEvent(NMPublishEvents event, EventDataVariant&& data)
+        {
+            LOG_ENTRY_FUNCTION();
+            {
+                std::lock_guard<std::mutex> lock(m_eventMutex);
+                m_eventQueue.push({event, std::move(data)});
+                NMLOG_DEBUG("Event %d queued, queue size: %zu", event, m_eventQueue.size());
+            }
+            m_eventCondVar.notify_one();
+        }


            /* Stop WiFi Signal Monitoring */
            stopWiFiSignalQualityMonitor();

+            /* Stop event dispatch thread */
+            {
+                std::unique_lock<std::mutex> lock(m_eventMutex);
+                m_eventThreadStop.store(true);


 endif ()

-set(PLUGIN_NETWORKMANAGER_LOGLEVEL "5" CACHE STRING "To configure default loglevel NetworkManager plugin")
+set(PLUGIN_NETWORKMANAGER_LOGLEVEL "3" CACHE STRING "To configure default loglevel NetworkManager plugin")


Copilot AI review requested due to automatic review settings July 6, 2026 21:09

karuna2git requested a review from a team as a code owner July 6, 2026 21:09

Copilot started reviewing on behalf of karuna2git July 6, 2026 21:09 View session

Copilot AI reviewed Jul 6, 2026

View reviewed changes

karuna2git force-pushed the topic/RDKEMW-20646-2 branch from dacaa00 to e0b4654 Compare July 6, 2026 21:22

Copilot AI review requested due to automatic review settings July 6, 2026 21:32

karuna2git force-pushed the topic/RDKEMW-20646-2 branch from e0b4654 to 98e1a45 Compare July 6, 2026 21:32

Copilot started reviewing on behalf of karuna2git July 6, 2026 21:32 View session

karuna2git force-pushed the topic/RDKEMW-20646-2 branch from 98e1a45 to 8111fe9 Compare July 6, 2026 21:34

Copilot AI reviewed Jul 6, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 6, 2026 21:37

Copilot started reviewing on behalf of karuna2git July 6, 2026 21:37 View session

karuna2git force-pushed the topic/RDKEMW-20646-2 branch from 8111fe9 to ebc2f9b Compare July 6, 2026 21:39

Copilot AI reviewed Jul 6, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 6, 2026 21:42

Copilot started reviewing on behalf of karuna2git July 6, 2026 21:43 View session

Copilot AI reviewed Jul 6, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 10, 2026 19:54

karuna2git force-pushed the topic/RDKEMW-20646-2 branch from ebc2f9b to b8dd5c6 Compare July 10, 2026 19:54

Copilot started reviewing on behalf of karuna2git July 10, 2026 19:55 View session

Copilot AI reviewed Jul 10, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 10, 2026 20:18

karuna2git force-pushed the topic/RDKEMW-20646-2 branch from b8dd5c6 to 7d7dc76 Compare July 10, 2026 20:18

Copilot started reviewing on behalf of karuna2git July 10, 2026 20:19 View session

Copilot AI reviewed Jul 10, 2026

View reviewed changes

Uh oh!

Conversation

karuna2git commented Jul 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants