Skip to content

RDKEMW-20574 : Fix the notification locks in networkmanager plugin #319

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
karuna2git merged 13 commits into from
Jul 1, 2026
Merged

RDKEMW-20574 : Fix the notification locks in networkmanager plugin #319

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
72184aa
Update NetworkManagerGnomeProxy.cpp
jincysam87 Jun 25, 2026
ec8fb0d
Update NetworkManagerRDKProxy.cpp
jincysam87 Jun 25, 2026
29c743a
Update NetworkManagerImplementation.cpp
jincysam87 Jun 25, 2026
d724ad6
Update NetworkManagerImplementation.h
jincysam87 Jun 25, 2026
d757a80
Update NetworkManagerImplementation.cpp
jincysam87 Jun 25, 2026
43abfe7
Update NetworkManagerImplementation.h
jincysam87 Jun 25, 2026
065578b
Update NetworkManagerLogger.h
jincysam87 Jun 25, 2026
369963a
Updated
jincysam87 Jun 30, 2026
23ab644
Potential fix for pull request finding
jincysam87 Jun 30, 2026
640d3e5
Updated
jincysam87 Jun 30, 2026
b997e7d
Apply suggestions from code review
jincysam87 Jun 30, 2026
8b03526
Updated review comments
jincysam87 Jun 30, 2026
a637eb2
Updated
jincysam87 Jun 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 25 additions & 14 deletions plugin/NetworkManagerImplementation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -597,14 +597,16 @@ namespace WPEFramework
return;
}

void NetworkManagerImplementation::filterScanResults(JsonArray &ssids)
void NetworkManagerImplementation::filterScanResults(JsonArray &ssids,
const std::vector<std::string>& filterSsidslist,
const std::vector<std::string>& filterFrequencies)
{
JsonArray result;
double filterFreq = 0.0;
std::unordered_set<std::string> scanForSsidsSet(m_filterSsidslist.begin(), m_filterSsidslist.end());
std::unordered_set<std::string> scanForSsidsSet(filterSsidslist.begin(), filterSsidslist.end());

// If neither SSID list nor frequency is provided, exit
if (m_filterSsidslist.empty() && m_filterFrequencies.empty())
if (filterSsidslist.empty() && filterFrequencies.empty())
{
NMLOG_DEBUG("Neither SSID nor Frequency is provided. Exiting function.");
return;
Expand All @@ -618,10 +620,10 @@ namespace WPEFramework

double frequencyValue = std::stod(frequency);
Comment thread
karuna2git marked this conversation as resolved.
bool ssidMatches = scanForSsidsSet.empty() || scanForSsidsSet.find(ssid) != scanForSsidsSet.end();
bool freqMatches = m_filterFrequencies.empty();
bool freqMatches = filterFrequencies.empty();
Comment thread
karuna2git marked this conversation as resolved.
if (!freqMatches)
{
Comment thread
karuna2git marked this conversation as resolved.
for (const auto& selectedFrequency : m_filterFrequencies)
for (const auto& selectedFrequency : filterFrequencies)
Comment thread
karuna2git marked this conversation as resolved.
{
if (selectedFrequency == "ALL")
{
Comment thread
karuna2git marked this conversation as resolved.
Comment thread
karuna2git marked this conversation as resolved.
Comment thread
jincysam87 marked this conversation as resolved.
Expand Down Expand Up @@ -738,7 +740,6 @@ namespace WPEFramework

void NetworkManagerImplementation::ReportActiveInterfaceChange(const string prevActiveInterface, const string currentActiveinterface)
{
_notificationLock.Lock();
NMLOG_INFO("Posting onActiveInterfaceChange %s", currentActiveinterface.c_str());

if(currentActiveinterface == "eth0")
Expand All @@ -754,15 +755,15 @@ namespace WPEFramework

// FIXME : This could be the place to define `m_defaultInterface` to incoming `currentActiveinterface`.
// m_defaultInterface = currentActiveinterface;

_notificationLock.Lock();
for (const auto callback : _notificationCallbacks) {
callback->onActiveInterfaceChange(prevActiveInterface, currentActiveinterface);
}
_notificationLock.Unlock();
#if USE_TELEMETRY
NMLOG_INFO("NM_INTERFACE_STATUS = Interface changed to %s", currentActiveinterface.c_str());
logTelemetry("NM_INTERFACE_STATUS", "Interface changed to " + currentActiveinterface);
#endif
_notificationLock.Unlock();
#endif
}

void NetworkManagerImplementation::ReportIPAddressChange(const string interface, const string ipversion, const string ipaddress, const Exchange::INetworkManager::IPStatus status)
Expand Down Expand Up @@ -810,7 +811,6 @@ namespace WPEFramework

void NetworkManagerImplementation::ReportInternetStatusChange(const Exchange::INetworkManager::InternetStatus prevState, const Exchange::INetworkManager::InternetStatus currState, const string interface)
{
_notificationLock.Lock();
NMLOG_INFO("Posting onInternetStatusChange with current state as %u", (unsigned)currState);
#if USE_TELEMETRY
// Log error only when ethernet is up and there's no internet
Expand All @@ -823,15 +823,16 @@ namespace WPEFramework
logTelemetry("NM_ETHERNET_CONNECTIVITY", "Ethernet connectivity failed");
}
#endif
_notificationLock.Lock();
for (const auto callback : _notificationCallbacks) {
callback->onInternetStatusChange(prevState, currState, interface);
}
_notificationLock.Unlock();
#if USE_TELEMETRY
string stateStr = Core::EnumerateType<Exchange::INetworkManager::InternetStatus>(currState).Data();
NMLOG_INFO("NM_INTERNET_STATUS = %s", stateStr.c_str());
logTelemetry("NM_INTERNET_STATUS", stateStr);
#endif
_notificationLock.Unlock();
}

int32_t NetworkManagerImplementation::logSSIDs(Logging level, const JsonArray &ssids)
Expand All @@ -855,7 +856,6 @@ namespace WPEFramework

void NetworkManagerImplementation::ReportAvailableSSIDs(const JsonArray &arrayofWiFiScanResults)
{
_notificationLock.Lock();
string jsonOfWiFiScanResults;
string jsonOfFilterScanResults;
JsonArray filterResult = arrayofWiFiScanResults;
Expand All @@ -864,12 +864,23 @@ namespace WPEFramework
NMLOG_DEBUG("Discovered %d SSIDs before filtering as,", filterResult.Length());
logSSIDs(LOG_LEVEL_DEBUG, filterResult);

filterScanResults(filterResult);
// Snapshot filter vectors under lock, then release before calling filterScanResults
// to ensure exception-safety (std::stod can throw).
std::vector<std::string> ssidsSnapshot;
std::vector<std::string> frequenciesSnapshot;
m_filterVectorsLock.Lock();
ssidsSnapshot = m_filterSsidslist;
frequenciesSnapshot = m_filterFrequencies;
m_filterVectorsLock.Unlock();

// Call filterScanResults outside the lock with snapshots (exception-safe)
filterScanResults(filterResult, ssidsSnapshot, frequenciesSnapshot);
filterResult.ToString(jsonOfFilterScanResults);

NMLOG_INFO("Posting onAvailableSSIDs event with %d SSIDs as,", filterResult.Length());
logSSIDs(LOG_LEVEL_INFO, filterResult);

_notificationLock.Lock();
for (const auto callback : _notificationCallbacks) {
callback->onAvailableSSIDs(jsonOfFilterScanResults);
}
Expand Down Expand Up @@ -1177,13 +1188,13 @@ namespace WPEFramework
m_wlanConnected.store(false); /* Any other state is considered as WiFi not connected. */
}

_notificationLock.Lock();
NMLOG_INFO("Posting onWiFiStateChange (%d)", state);
#if USE_TELEMETRY
string stateStr = Core::EnumerateType<Exchange::INetworkManager::WiFiState>(state).Data();
NMLOG_INFO("NM_WIFI_STATUS = %s", stateStr.c_str());
logTelemetry("NM_WIFI_STATUS", stateStr);
#endif
_notificationLock.Lock();
for (const auto callback : _notificationCallbacks) {
callback->onWiFiStateChange(state);
}
Expand Down
3 changes: 2 additions & 1 deletion plugin/NetworkManagerImplementation.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ namespace WPEFramework
void getInitialConnectionState(void);
void executeExternally(NetworkEvents event, const string commandToExecute, string& response);
void threadEventRegistration(bool iarmInit, bool iarmConnect);
void filterScanResults(JsonArray &ssids);
void filterScanResults(JsonArray &ssids, const std::vector<std::string>& filterSsidslist, const std::vector<std::string>& filterFrequencies);
void startWiFiSignalQualityMonitor(int interval);
void stopWiFiSignalQualityMonitor();
void monitorThreadFunction(int interval);
Expand All @@ -348,6 +348,7 @@ namespace WPEFramework
private:
std::list<Exchange::INetworkManager::INotification *> _notificationCallbacks;
Core::CriticalSection _notificationLock;
Core::CriticalSection m_filterVectorsLock;
string m_publicIP;
stun::client stunClient;
string m_stunEndpoint;
Expand Down
2 changes: 1 addition & 1 deletion plugin/NetworkManagerLogger.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ void logPrint(LogLevel level, const char* file, const char* func, int line, cons
#define NMLOG_ERROR(FMT, ...) logPrint(NetworkManagerLogger::ERROR_LEVEL, __FILE__, __func__, __LINE__, FMT, ##__VA_ARGS__)
#define NMLOG_FATAL(FMT, ...) logPrint(NetworkManagerLogger::FATAL_LEVEL, __FILE__,__func__, __LINE__, FMT, ##__VA_ARGS__)

#define LOG_ENTRY_FUNCTION() { NMLOG_DEBUG("Entering"); }
#define LOG_ENTRY_FUNCTION() { NMLOG_INFO("Entering %s", __func__); }
Comment thread
jincysam87 marked this conversation as resolved.

} // namespace NetworkManagerLogger

Expand Down
19 changes: 13 additions & 6 deletions plugin/gnome/NetworkManagerGnomeProxy.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -762,9 +762,8 @@ namespace WPEFramework
{
uint32_t rc = Core::ERROR_RPC_CALL_FAILED;

//Cleared the Existing Store filterred SSID list
m_filterSsidslist.clear();
m_filterFrequencies.clear();
std::vector<std::string> filteredSsids;
std::vector<std::string> filteredFrequencies;

if(ssids)
{
Expand All @@ -773,7 +772,7 @@ namespace WPEFramework
{
if (!tmpssidlist.empty())
{
m_filterSsidslist.push_back(tmpssidlist.c_str());
filteredSsids.push_back(tmpssidlist);
NMLOG_DEBUG("%s added to SSID filtering", tmpssidlist.c_str());
}
else
Expand All @@ -795,7 +794,7 @@ namespace WPEFramework
const string normalizedFrequency = parsedFrequency.Data();
if ((!normalizedFrequency.empty()) && (normalizedFrequency == frequency))
{
m_filterFrequencies.push_back(normalizedFrequency);
filteredFrequencies.push_back(normalizedFrequency);
NMLOG_DEBUG("Frequency %s added to scan filtering", normalizedFrequency.c_str());
}
else
Expand All @@ -811,8 +810,16 @@ namespace WPEFramework
}
}

m_filterVectorsLock.Lock();
// Replace existing stored filters only after successful parsing/validation.
m_filterSsidslist.clear();
m_filterFrequencies.clear();
m_filterSsidslist = filteredSsids;
m_filterFrequencies = filteredFrequencies;
m_filterVectorsLock.Unlock();
Comment on lines +813 to +819

@karuna2git karuna2git Jul 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since filteredSsids is still needed after updating m_filterSsidslist, then do not use std::move() or std::swap() because the contents of filteredSsids become unspecified.

C++m_filterSsidslist = filteredSsids; This is the clearest and safest option.


nmEvent->setwifiScanOptions(true);
if(wifi->wifiScanRequest(m_filterSsidslist))
if(wifi->wifiScanRequest(filteredSsids))
rc = Core::ERROR_NONE;
return rc;
}
Expand Down
2 changes: 2 additions & 0 deletions plugin/rdk/NetworkManagerRDKProxy.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -973,6 +973,7 @@ const string CIDR_PREFIXES[CIDR_NETMASK_IP_LEN+1] = {
IARM_Result_t retVal = IARM_RESULT_SUCCESS;

//Cleared the Existing Store filterred SSID list
m_filterVectorsLock.Lock();
m_filterSsidslist.clear();
m_filterFrequencies.clear();
if(ssids)
Expand All @@ -994,6 +995,7 @@ const string CIDR_PREFIXES[CIDR_NETMASK_IP_LEN+1] = {
NMLOG_DEBUG("%s added to Frequency filtering", frequencyList.c_str());
}
}
m_filterVectorsLock.Unlock();
Comment thread
karuna2git marked this conversation as resolved.

memset(&param, 0, sizeof(param));

Expand Down
Loading