From 60d62cb9f92d47a40bbf23ffab37959537fac36b Mon Sep 17 00:00:00 2001
From: Mohamed Chibani <mohamed.chibani@savoirfairelinux.com>
Date: Mon, 1 Feb 2021 09:46:45 -0500
Subject: [PATCH] upnp: ignore callbacks from PUPNP lib if the client is not
registered
A callback from PUPNP lib may be received during the shutdown of the
daemon.
To prevent this race, we lock the ctrl point and we ignore the callback
if the client is registered.
Change-Id: Ie6a3cb37fc189a271673e0935a5f03abce1ffc73
Gitlab: #335
---
src/upnp/protocol/pupnp/pupnp.cpp | 12 +++++++++++-
src/upnp/protocol/pupnp/pupnp.h | 7 +++++--
2 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/src/upnp/protocol/pupnp/pupnp.cpp b/src/upnp/protocol/pupnp/pupnp.cpp
index c6779a5ad1..2be07ee59d 100644
--- a/src/upnp/protocol/pupnp/pupnp.cpp
+++ b/src/upnp/protocol/pupnp/pupnp.cpp
@@ -94,6 +94,9 @@ errorOnResponse(IXML_Document* doc)
return false;
}
+
+// UPNP class implementation
+
PUPnP::PUPnP()
{
if (not initUpnpLib())
@@ -168,6 +171,7 @@ PUPnP::~PUPnP()
removeAllLocalMappings(it);
}
validIgdList_.clear();
+ clientRegistered_ = false;
UpnpUnRegisterClient(ctrlptHandle_);
}
@@ -646,8 +650,10 @@ PUPnP::eventTypeToString(Upnp_EventType eventType)
int
PUPnP::ctrlPtCallback(Upnp_EventType event_type, const void* event, void* user_data)
{
- if (auto pupnp = static_cast<PUPnP*>(user_data))
+ if (auto pupnp = static_cast<PUPnP*>(user_data)) {
+ std::lock_guard<std::mutex> lk(pupnp->ctrlptMutex_);
return pupnp->handleCtrlPtUPnPEvents(event_type, event);
+ }
JAMI_WARN("PUPnP: Control point callback without PUPnP");
return 0;
@@ -786,6 +792,10 @@ PUPnP::processDiscoverySubscriptionExpired(const std::string& eventSubUrl)
int
PUPnP::handleCtrlPtUPnPEvents(Upnp_EventType event_type, const void* event)
{
+ // Ignore if not registered
+ if (not PUPnP::clientRegistered_)
+ return UPNP_E_SUCCESS;
+
switch (event_type) {
// "ALIVE" events are processed as "SEARCH RESULT". It might be usefull
// if "SEARCH RESULT" was missed.
diff --git a/src/upnp/protocol/pupnp/pupnp.h b/src/upnp/protocol/pupnp/pupnp.h
index cdcb8594c4..648b15f138 100644
--- a/src/upnp/protocol/pupnp/pupnp.h
+++ b/src/upnp/protocol/pupnp/pupnp.h
@@ -214,6 +214,10 @@ private:
static const char* eventTypeToString(Upnp_EventType eventType);
private:
+ // Indicates of the client is registered. Since at most only one instance
+ // of this class is created, using a static is unambiguous.
+ std::atomic_bool clientRegistered_ {false};
+
std::weak_ptr<PUPnP> weak() { return std::static_pointer_cast<PUPnP>(shared_from_this()); }
NON_COPYABLE(PUPnP);
@@ -235,8 +239,7 @@ private:
std::mutex ctrlptMutex_; // Mutex for client handle protection.
UpnpClient_Handle ctrlptHandle_ {-1}; // Control point handle.
- std::atomic_bool clientRegistered_ {false}; // Indicates of the client is registered.
- std::atomic_bool searchForIgd_ {false}; // Variable to signal thread for a search.
+ std::atomic_bool searchForIgd_ {false}; // Variable to signal thread for a search.
};
} // namespace upnp
--
GitLab