diff --git a/src/manager.cpp b/src/manager.cpp
index 7ce39ed1da5245b9894f7ea576366b32b90a0ae2..3c135388058be9d3d554eb05fab4b7de47a60c57 100644
--- a/src/manager.cpp
+++ b/src/manager.cpp
@@ -91,6 +91,8 @@ using random_device = dht::crypto::random_device;
#include "data_transfer.h"
#include "dring/media_const.h"
+#include "upnp/upnp_context.h"
+
#include <libavutil/ffversion.h>
#include <opendht/thread_pool.h>
@@ -845,6 +847,9 @@ Manager::finish() noexcept
return;
try {
+ // Terminate UPNP context
+ jami::upnp::UPnPContext::getUPnPContext()->shutdown();
+
// Forbid call creation
callFactory.forbid();
@@ -894,6 +899,7 @@ Manager::finish() noexcept
}
if (pimpl_->ioContextRunner_.joinable())
pimpl_->ioContextRunner_.join();
+
} catch (const VoipLinkException& err) {
JAMI_ERR("%s", err.what());
}
@@ -2828,7 +2834,6 @@ Manager::removeAccounts()
removeAccount(acc);
}
-
std::vector<std::string_view>
Manager::loadAccountOrder() const
{
diff --git a/src/upnp/protocol/natpmp/nat_pmp.cpp b/src/upnp/protocol/natpmp/nat_pmp.cpp
index eab6049fe32e650ce29a3dc209b0b41fbf12df99..c7d20f890549359d92701f6a76d79f6793b6c3fa 100644
--- a/src/upnp/protocol/natpmp/nat_pmp.cpp
+++ b/src/upnp/protocol/natpmp/nat_pmp.cpp
@@ -27,6 +27,7 @@ namespace upnp {
NatPmp::NatPmp()
{
+ JAMI_DBG("NAT-PMP: Instance [%p] created", this);
getNatpmpScheduler()->run([this] {
threadId_ = getCurrentThread();
igd_ = std::make_shared<PMPIGD>();
@@ -35,7 +36,7 @@ NatPmp::NatPmp()
NatPmp::~NatPmp()
{
- clearIgds();
+ JAMI_DBG("NAT-PMP: Instance [%p] destroyed", this);
}
void
@@ -46,8 +47,6 @@ NatPmp::initNatPmp()
return;
}
- CHECK_VALID_THREAD();
-
initialized_ = false;
hostAddress_ = ip_utils::getLocalAddr(pj_AF_INET());
@@ -116,10 +115,27 @@ NatPmp::initNatPmp()
void
NatPmp::setObserver(UpnpMappingObserver* obs)
{
+ if (not isValidThread()) {
+ getNatpmpScheduler()->run([this, obs] { setObserver(obs); });
+ return;
+ }
+
JAMI_DBG("NAT-PMP: Setting observer to %p", obs);
+
observer_ = obs;
}
+void
+NatPmp::terminate()
+{
+ if (not isValidThread()) {
+ getNatpmpScheduler()->run([this] { terminate(); });
+ return;
+ }
+
+ observer_ = nullptr;
+}
+
void
NatPmp::clearIgds()
{
@@ -128,8 +144,6 @@ NatPmp::clearIgds()
return;
}
- CHECK_VALID_THREAD();
-
initialized_ = false;
if (searchForIgdTimer_)
searchForIgdTimer_->cancel();
@@ -148,8 +162,6 @@ NatPmp::searchForIgd()
return;
}
- CHECK_VALID_THREAD();
-
if (not initialized_) {
initNatPmp();
}
@@ -180,6 +192,11 @@ NatPmp::getIgdList(std::list<std::shared_ptr<IGD>>& igdList) const
bool
NatPmp::isReady() const
{
+ if (observer_ == nullptr) {
+ JAMI_ERR("NAT-PMP: the observer is not set!");
+ return false;
+ }
+
// Must at least have a valid local address.
if (not getHostAddress() or getHostAddress().isLoopback())
return false;
@@ -616,6 +633,8 @@ NatPmp::processIgdUpdate(UpnpIgdEvent event)
removeAllMappings();
}
+ if (observer_ == nullptr)
+ return;
// Process the response on the context thread.
runOnUpnpContextThread([obs = observer_, igd = igd_, event] { obs->onIgdUpdated(igd, event); });
}
@@ -623,6 +642,9 @@ NatPmp::processIgdUpdate(UpnpIgdEvent event)
void
NatPmp::processMappingAdded(const Mapping& map)
{
+ if (observer_ == nullptr)
+ return;
+
// Process the response on the context thread.
runOnUpnpContextThread([obs = observer_, igd = igd_, map] { obs->onMappingAdded(igd, map); });
}
@@ -630,6 +652,9 @@ NatPmp::processMappingAdded(const Mapping& map)
void
NatPmp::processMappingRenewed(const Mapping& map)
{
+ if (observer_ == nullptr)
+ return;
+
// Process the response on the context thread.
runOnUpnpContextThread([obs = observer_, igd = igd_, map] { obs->onMappingRenewed(igd, map); });
}
@@ -637,6 +662,9 @@ NatPmp::processMappingRenewed(const Mapping& map)
void
NatPmp::processMappingRemoved(const Mapping& map)
{
+ if (observer_ == nullptr)
+ return;
+
// Process the response on the context thread.
runOnUpnpContextThread([obs = observer_, igd = igd_, map] { obs->onMappingRemoved(igd, map); });
}
diff --git a/src/upnp/protocol/natpmp/nat_pmp.h b/src/upnp/protocol/natpmp/nat_pmp.h
index ddf0a7ea8bfa40916b321a826cc7ca565625d150..fe2bf22956771f54db6a64f2621f9bc7346e9336 100644
--- a/src/upnp/protocol/natpmp/nat_pmp.h
+++ b/src/upnp/protocol/natpmp/nat_pmp.h
@@ -97,6 +97,10 @@ public:
// Removes a mapping.
void requestMappingRemove(const Mapping& igdMapping) override;
+ // Terminate. Nothing to do here, the clean-up is done when
+ // the IGD is cleared.
+ void terminate() override;
+
private:
void initNatPmp();
void getIgdPublicAddress();
diff --git a/src/upnp/protocol/pupnp/pupnp.cpp b/src/upnp/protocol/pupnp/pupnp.cpp
index 0b81a8c0992f3a1ad565501cb274d7d146d5b550..20aef2bede6dd725f73f1941c10f8ef8491c3f68 100644
--- a/src/upnp/protocol/pupnp/pupnp.cpp
+++ b/src/upnp/protocol/pupnp/pupnp.cpp
@@ -98,20 +98,90 @@ errorOnResponse(IXML_Document* doc)
PUPnP::PUPnP()
{
+ JAMI_DBG("PUPnP: Instance [%p] created", this);
threadId_ = getCurrentThread();
+}
- if (not initUpnpLib())
- return;
- if (not registerClient())
+PUPnP::~PUPnP()
+{
+ JAMI_DBG("PUPnP: Instance [%p] destroyed", this);
+}
+
+void
+PUPnP::initUpnpLib()
+{
+ assert(not initialized_);
+
+ int upnp_err = UpnpInit2(nullptr, 0);
+
+ if (upnp_err != UPNP_E_SUCCESS) {
+ JAMI_ERR("PUPnP: Can't initialize libupnp: %s", UpnpGetErrorMessage(upnp_err));
+ UpnpFinish();
+ initialized_ = false;
return;
+ }
+
+ // Disable embedded WebServer if any.
+ if (UpnpIsWebserverEnabled() == 1) {
+ JAMI_WARN("PUPnP: Web-server is enabled. Disabling");
+ UpnpEnableWebserver(0);
+ if (UpnpIsWebserverEnabled() == 1) {
+ JAMI_ERR("PUPnP: Could not disable Web-server!");
+ } else {
+ JAMI_DBG("PUPnP: Web-server successfully disabled");
+ }
+ }
+
+ char* ip_address = UpnpGetServerIpAddress();
+ char* ip_address6 = nullptr;
+ unsigned short port = UpnpGetServerPort();
+ unsigned short port6 = 0;
+#if UPNP_ENABLE_IPV6
+ ip_address6 = UpnpGetServerIp6Address();
+ port6 = UpnpGetServerPort6();
+#endif
+ if (ip_address6 and port6)
+ JAMI_DBG("PUPnP: Initialized on %s:%u | %s:%u", ip_address, port, ip_address6, port6);
+ else
+ JAMI_DBG("PUPnP: Initialized on %s:%u", ip_address, port);
+
+ // Relax the parser to allow malformed XML text.
+ ixmlRelaxParser(1);
+
+ initialized_ = true;
+}
+
+void
+PUPnP::registerClient()
+{
+ assert(not clientRegistered_);
+
+ CHECK_VALID_THREAD();
+
+ // Register Upnp control point.
+ std::unique_lock<std::mutex> lk(ctrlptMutex_);
+ int upnp_err = UpnpRegisterClient(ctrlPtCallback, this, &ctrlptHandle_);
+ if (upnp_err != UPNP_E_SUCCESS) {
+ JAMI_ERR("PUPnP: Can't register client: %s", UpnpGetErrorMessage(upnp_err));
+ } else {
+ JAMI_DBG("PUPnP: Successfully registered client");
+ clientRegistered_ = true;
+ }
+}
+
+void
+PUPnP::startPUPnP()
+{
+ JAMI_DBG("PUPnP: Starting PUPNP internal thread");
pupnpThread_ = std::thread([this] {
+ JAMI_DBG("PUPnP: Internal thread started");
+ pupnpRun_ = true;
while (pupnpRun_) {
{
std::unique_lock<std::mutex> lk(igdListMutex_);
pupnpCv_.wait(lk, [this] {
- return not clientRegistered_ or not pupnpRun_ or searchForIgd_
- or not dwnldlXmlList_.empty();
+ return not pupnpRun_ or searchForIgd_ or not dwnldlXmlList_.empty();
});
}
@@ -158,90 +228,60 @@ PUPnP::PUPnP()
}
}
}
+
+ JAMI_DBG("PUPnP: Internal thread stopped");
});
}
-PUPnP::~PUPnP()
+void
+PUPnP::setObserver(UpnpMappingObserver* obs)
{
- // Clear all the lists.
+ CHECK_VALID_THREAD();
+
+ JAMI_DBG("PUPnP: Setting observer to %p", obs);
+
+ observer_ = obs;
+}
+
+void
+PUPnP::terminate()
+{
+ CHECK_VALID_THREAD();
+
+ JAMI_DBG("PUPnP: Terminate instance %p", this);
+
+ clientRegistered_ = false;
+ observer_ = nullptr;
+
{
std::lock_guard<std::mutex> lock(validIgdListMutex_);
std::lock_guard<std::mutex> lk(ctrlptMutex_);
- validIgdList_.clear();
- clientRegistered_ = false;
+
UpnpUnRegisterClient(ctrlptHandle_);
}
- {
- std::lock_guard<std::mutex> lk2(igdListMutex_);
- discoveredIgdList_.clear();
- dwnldlXmlList_.clear();
- cancelXmlList_.clear();
- pupnpRun_ = false;
+ if (UpnpFinish() != UPNP_E_SUCCESS) {
+ JAMI_ERR("PUPnP: Failed to properly close lib-upnp");
}
- UpnpFinish();
-
+ pupnpRun_ = false;
// Notify thread to terminate.
pupnpCv_.notify_all();
if (pupnpThread_.joinable())
pupnpThread_.join();
- JAMI_DBG("PUPnP: Instance [%p] destroyed", this);
-}
-
-bool
-PUPnP::initUpnpLib()
-{
- int upnp_err = UpnpInit2(nullptr, 0);
-
- if (upnp_err != UPNP_E_SUCCESS) {
- JAMI_ERR("PUPnP: Can't initialize libupnp: %s", UpnpGetErrorMessage(upnp_err));
- UpnpFinish();
- pupnpRun_ = false;
- return false;
+ // Clear all the lists.
+ {
+ std::lock_guard<std::mutex> lock(validIgdListMutex_);
+ validIgdList_.clear();
}
- char* ip_address = UpnpGetServerIpAddress();
- char* ip_address6 = nullptr;
- unsigned short port = UpnpGetServerPort();
- unsigned short port6 = 0;
-#if UPNP_ENABLE_IPV6
- ip_address6 = UpnpGetServerIp6Address();
- port6 = UpnpGetServerPort6();
-#endif
- if (ip_address6 and port6)
- JAMI_DBG("PUPnP: Initialized on %s:%u | %s:%u", ip_address, port, ip_address6, port6);
- else
- JAMI_DBG("PUPnP: Initialized on %s:%u", ip_address, port);
- // Relax the parser to allow malformed XML text.
- ixmlRelaxParser(1);
-
- return true;
-}
-
-bool
-PUPnP::registerClient()
-{
- // Register Upnp control point.
- std::unique_lock<std::mutex> lk(ctrlptMutex_);
- int upnp_err = UpnpRegisterClient(ctrlPtCallback, this, &ctrlptHandle_);
- if (upnp_err != UPNP_E_SUCCESS) {
- JAMI_ERR("PUPnP: Can't register client: %s", UpnpGetErrorMessage(upnp_err));
- pupnpRun_ = false;
- return false;
- } else {
- JAMI_DBG("PUPnP: Successfully registered client");
- clientRegistered_ = true;
+ {
+ std::lock_guard<std::mutex> lk2(igdListMutex_);
+ discoveredIgdList_.clear();
+ dwnldlXmlList_.clear();
+ cancelXmlList_.clear();
}
-
- return true;
-}
-
-void
-PUPnP::setObserver(UpnpMappingObserver* obs)
-{
- observer_ = obs;
}
void
@@ -295,25 +335,58 @@ PUPnP::searchForIgd()
// Update local address before searching.
hostAddress_ = ip_utils::getLocalAddr(pj_AF_INET());
- if (not isReady()) {
- if (igdSearchCounter_++ < PUPNP_MAX_RESTART_SEARCH_RETRIES) {
- JAMI_DBG("PUPnP: Start search for IGD: attempt %u", igdSearchCounter_);
+ if (isReady()) {
+ JAMI_WARN("PUPnP: Already have a valid IGD. Skipping the search request");
+ return;
+ }
- searchForIgd_ = true;
- pupnpCv_.notify_one();
+ if (igdSearchCounter_++ >= PUPNP_MAX_RESTART_SEARCH_RETRIES) {
+ JAMI_WARN("PUPnP: Setup failed after %u trials. PUPnP will be disabled!",
+ PUPNP_MAX_RESTART_SEARCH_RETRIES);
+ return;
+ }
- // Cancel the current timer (if any) and re-schedule.
- if (searchForIgdTimer_)
- searchForIgdTimer_->cancel();
+ JAMI_DBG("PUPnP: Start search for IGD: attempt %u", igdSearchCounter_);
- searchForIgdTimer_ = getUpnContextScheduler()
- ->scheduleIn([this] { searchForIgd(); },
- PUPNP_TIMEOUT_BEFORE_IGD_SEARCH_RETRY);
- } else {
- JAMI_WARN("PUPnP: Setup failed after %u trials. PUPnP will be disabled!",
- PUPNP_MAX_RESTART_SEARCH_RETRIES);
+ if (not pupnpRun_) {
+ startPUPnP();
+ }
+
+ // Do not init if the host is not valid. Otherwise, the init will fail
+ // anyway and may put libupnp in an unstable state (mainly deadlocks)
+ // even if the UpnpFinish() method is called.
+ if (not hostAddress_ or hostAddress_.isLoopback()) {
+ JAMI_WARN("PUPnP: Host address is invalid. Skipping the IGD search");
+ } else {
+ // Init and register if needed
+ if (not initialized_) {
+ initUpnpLib();
+ }
+ if (initialized_ and not clientRegistered_) {
+ registerClient();
}
}
+
+ // Start search
+ if (clientRegistered_ and pupnpRun_) {
+ assert(initialized_);
+ JAMI_DBG("PUPnP: Start search for IGD");
+ searchForIgd_ = true;
+ pupnpCv_.notify_one();
+ } else {
+ JAMI_WARN("PUPnP: PUPNP not fully setup. Skipping the IGD search");
+ }
+
+ // Cancel the current timer (if any) and re-schedule.
+ if (searchForIgdTimer_)
+ searchForIgdTimer_->cancel();
+
+ searchForIgdTimer_ = getUpnContextScheduler()->scheduleIn(
+ [w = weak()] {
+ if (auto upnpThis = w.lock())
+ upnpThis->searchForIgd();
+ },
+ PUPNP_TIMEOUT_BEFORE_IGD_SEARCH_RETRY);
}
void
@@ -369,7 +442,8 @@ PUPnP::incrementErrorsCounter(const std::shared_ptr<IGD>& igd)
// Disable this IGD.
igd->setValid(false);
// Notify the listener.
- observer_->onIgdUpdated(igd, UpnpIgdEvent::INVALID_STATE);
+ if (observer_)
+ observer_->onIgdUpdated(igd, UpnpIgdEvent::INVALID_STATE);
}
}
@@ -484,18 +558,24 @@ PUPnP::validateIgd(const IGDInfo& info)
deleteMappingsByDescription(igd_candidate, Mapping::UPNP_MAPPING_DESCRIPTION_PREFIX);
// Report to the listener.
- observer_->onIgdUpdated(igd_candidate, UpnpIgdEvent::ADDED);
+ if (observer_)
+ observer_->onIgdUpdated(igd_candidate, UpnpIgdEvent::ADDED);
// Subscribe to IGD events.
int upnp_err = UpnpSubscribeAsync(ctrlptHandle_,
eventSub.c_str(),
- SUBSCRIBE_TIMEOUT,
+ UPNP_INFINITE,
subEventCallback,
this);
- if (upnp_err != UPNP_E_SUCCESS)
- JAMI_WARN("PUPnP: Error when trying to request subscription for %s -> %s",
+ if (upnp_err != UPNP_E_SUCCESS) {
+ JAMI_WARN("PUPnP: Failed to send subscribe request to %s: error %i - %s",
igd_candidate->getUID().c_str(),
+ upnp_err,
UpnpGetErrorMessage(upnp_err));
+ } else {
+ JAMI_DBG("PUPnP: Successfully sent subscribe request to %s",
+ igd_candidate->getUID().c_str());
+ }
return true;
}
@@ -503,13 +583,16 @@ PUPnP::validateIgd(const IGDInfo& info)
void
PUPnP::requestMappingAdd(const std::shared_ptr<IGD>& igd, const Mapping& mapping)
{
- if (auto pupnp_igd = std::dynamic_pointer_cast<const UPnPIGD>(igd)) {
- actionAddPortMappingAsync(*pupnp_igd, mapping);
+ if (auto pupnp_igd = std::dynamic_pointer_cast<UPnPIGD>(igd)) {
+ dht::ThreadPool::io().run([w = weak(), pupnp_igd, mapping] {
+ if (auto upnpThis = w.lock())
+ upnpThis->actionAddPortMapping(pupnp_igd, mapping);
+ });
}
}
bool
-PUPnP::actionAddPortMappingAsync(const UPnPIGD& igd, const Mapping& mapping)
+PUPnP::actionAddPortMappingAsync(const std::shared_ptr<UPnPIGD>& igd, const Mapping& mapping)
{
if (not clientRegistered_) {
return false;
@@ -519,48 +602,49 @@ PUPnP::actionAddPortMappingAsync(const UPnPIGD& igd, const Mapping& mapping)
// Set action sequence.
UpnpAddToAction(&action_container_ptr,
ACTION_ADD_PORT_MAPPING,
- igd.getServiceType().c_str(),
+ igd->getServiceType().c_str(),
"NewRemoteHost",
"");
UpnpAddToAction(&action_container_ptr,
ACTION_ADD_PORT_MAPPING,
- igd.getServiceType().c_str(),
+ igd->getServiceType().c_str(),
"NewExternalPort",
mapping.getExternalPortStr().c_str());
UpnpAddToAction(&action_container_ptr,
ACTION_ADD_PORT_MAPPING,
- igd.getServiceType().c_str(),
+ igd->getServiceType().c_str(),
"NewProtocol",
mapping.getTypeStr());
UpnpAddToAction(&action_container_ptr,
ACTION_ADD_PORT_MAPPING,
- igd.getServiceType().c_str(),
+ igd->getServiceType().c_str(),
"NewInternalPort",
mapping.getInternalPortStr().c_str());
UpnpAddToAction(&action_container_ptr,
ACTION_ADD_PORT_MAPPING,
- igd.getServiceType().c_str(),
+ igd->getServiceType().c_str(),
"NewInternalClient",
getHostAddress().toString().c_str());
UpnpAddToAction(&action_container_ptr,
ACTION_ADD_PORT_MAPPING,
- igd.getServiceType().c_str(),
+ igd->getServiceType().c_str(),
"NewEnabled",
"1");
UpnpAddToAction(&action_container_ptr,
ACTION_ADD_PORT_MAPPING,
- igd.getServiceType().c_str(),
+ igd->getServiceType().c_str(),
"NewPortMappingDescription",
mapping.toString().c_str());
UpnpAddToAction(&action_container_ptr,
ACTION_ADD_PORT_MAPPING,
- igd.getServiceType().c_str(),
+ igd->getServiceType().c_str(),
"NewLeaseDuration",
"0");
action.reset(action_container_ptr);
+
int upnp_err = UpnpSendActionAsync(ctrlptHandle_,
- igd.getControlURL().c_str(),
- igd.getServiceType().c_str(),
+ igd->getControlURL().c_str(),
+ igd->getServiceType().c_str(),
nullptr,
action.get(),
ctrlPtCallback,
@@ -568,12 +652,14 @@ PUPnP::actionAddPortMappingAsync(const UPnPIGD& igd, const Mapping& mapping)
if (upnp_err != UPNP_E_SUCCESS) {
JAMI_WARN("PUPnP: Failed to send async action %s from: %s, %d: %s",
ACTION_ADD_PORT_MAPPING,
- igd.getServiceType().c_str(),
+ igd->getServiceType().c_str(),
upnp_err,
UpnpGetErrorMessage(upnp_err));
return false;
}
+
JAMI_DBG("PUPnP: Sent request to open port %s", mapping.toString().c_str());
+
return true;
}
@@ -585,8 +671,6 @@ PUPnP::processAddMapAction(const std::string& ctrlURL,
{
Mapping mapToAdd(portType, ePort, iPort);
- JAMI_DBG("PUPnP: Opened port %s", mapToAdd.toString().c_str());
-
{
std::lock_guard<std::mutex> lock(validIgdListMutex_);
for (auto const& it : validIgdList_) {
@@ -601,9 +685,11 @@ PUPnP::processAddMapAction(const std::string& ctrlURL,
}
if (mapToAdd.getIgd()) {
- observer_->onMappingAdded(mapToAdd.getIgd(), std::move(mapToAdd));
+ JAMI_DBG("PUPnP: Opened port %s", mapToAdd.toString().c_str());
+ if (observer_)
+ observer_->onMappingAdded(mapToAdd.getIgd(), std::move(mapToAdd));
} else {
- JAMI_WARN("PUPnP: Did not find matching ctrl URL [%s] for (map %s)",
+ JAMI_WARN("PUPnP: Did not find matching ctrl URL [%s] for %s",
ctrlURL.c_str(),
mapToAdd.toString().c_str());
}
@@ -616,6 +702,7 @@ PUPnP::processRemoveMapAction(const std::string& ctrlURL,
PortType portType)
{
Mapping mapToRemove(portType, ePort, iPort);
+
{
std::lock_guard<std::mutex> lock(validIgdListMutex_);
for (auto const& it : validIgdList_) {
@@ -629,26 +716,34 @@ PUPnP::processRemoveMapAction(const std::string& ctrlURL,
}
if (mapToRemove.getIgd()) {
- observer_->onMappingRemoved(mapToRemove.getIgd(), std::move(mapToRemove));
+ JAMI_DBG("PUPnP: Closed port %s", mapToRemove.toString().c_str());
+ if (observer_)
+ observer_->onMappingRemoved(mapToRemove.getIgd(), std::move(mapToRemove));
} else {
- JAMI_WARN("PUPnP: Did not find matching ctrl URL [%s] for (map %s)",
+ JAMI_WARN("PUPnP: Did not find matching ctrl URL [%s] for %s",
ctrlURL.c_str(),
mapToRemove.toString().c_str());
}
}
void
-PUPnP::requestMappingRemove(const Mapping& igdMapping)
+PUPnP::requestMappingRemove(const Mapping& mapping)
{
+ auto igd = std::dynamic_pointer_cast<UPnPIGD>(mapping.getIgd());
+
+ if (not igd)
+ return;
+
std::lock_guard<std::mutex> lock(validIgdListMutex_);
- // Iterate over all IGDs in internal list and try to remove selected mapping.
for (auto const& it : validIgdList_) {
- if (auto igd = std::dynamic_pointer_cast<UPnPIGD>(it)) {
- std::lock_guard<std::mutex> lk2(ctrlptMutex_);
- actionDeletePortMappingAsync(*igd,
- igdMapping.getExternalPortStr(),
- igdMapping.getTypeStr());
- return;
+ if (std::dynamic_pointer_cast<UPnPIGD>(it) == igd) {
+ // Send remove request using the matching IGD
+ dht::ThreadPool::io().run([w = weak(), igd, mapping] {
+ if (auto upnpThis = w.lock()) {
+ upnpThis->actionDeletePortMapping(igd, mapping);
+ }
+ });
+ break;
}
}
}
@@ -695,13 +790,24 @@ 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)) {
- std::lock_guard<std::mutex> lk(pupnp->ctrlptMutex_);
- return pupnp->handleCtrlPtUPnPEvents(event_type, event);
+ auto pupnp = static_cast<PUPnP*>(user_data);
+
+ if (pupnp == nullptr) {
+ JAMI_WARN("PUPnP: Control point callback without PUPnP");
+ return UPNP_E_SUCCESS;
}
- JAMI_WARN("PUPnP: Control point callback without PUPnP");
- return 0;
+ auto upnpThis = pupnp->weak().lock();
+
+ if (not upnpThis)
+ return UPNP_E_SUCCESS;
+
+ // Ignore if already unregistered.
+ if (not upnpThis->clientRegistered_)
+ return UPNP_E_SUCCESS;
+
+ // Process the callback.
+ return upnpThis->handleCtrlPtUPnPEvents(event_type, event);
}
PUPnP::CtrlAction
@@ -760,27 +866,27 @@ PUPnP::processDiscoverySearchResult(const std::string& cpDeviceId,
return;
}
- dwnldlXmlList_.emplace_back(
- dht::ThreadPool::io().get<pIGDInfo>([this, location = std::move(igdLocationUrl)] {
- IXML_Document* doc_container_ptr = nullptr;
- XMLDocument doc_desc_ptr(nullptr, ixmlDocument_free);
- int upnp_err = UpnpDownloadXmlDoc(location.c_str(), &doc_container_ptr);
- if (doc_container_ptr)
- doc_desc_ptr.reset(doc_container_ptr);
- pupnpCv_.notify_all();
- if (upnp_err != UPNP_E_SUCCESS or not doc_desc_ptr) {
- JAMI_WARN("PUPnP: Error downloading device XML document from %s -> %s",
- location.c_str(),
- UpnpGetErrorMessage(upnp_err));
- return std::make_unique<IGDInfo>(
- IGDInfo {std::move(location), XMLDocument(nullptr, ixmlDocument_free)});
- } else {
- JAMI_DBG("PUPnP: Succeeded to download device XML document from %s",
- location.c_str());
- return std::make_unique<IGDInfo>(
- IGDInfo {std::move(location), std::move(doc_desc_ptr)});
- }
- }));
+ dwnldlXmlList_.emplace_back(dht::ThreadPool::io().get<pIGDInfo>([this,
+ location = std::move(
+ igdLocationUrl)] {
+ IXML_Document* doc_container_ptr = nullptr;
+ XMLDocument doc_desc(nullptr, ixmlDocument_free);
+ int upnp_err = UpnpDownloadXmlDoc(location.c_str(), &doc_container_ptr);
+ doc_desc.reset(doc_container_ptr);
+
+ pupnpCv_.notify_all();
+
+ if (upnp_err != UPNP_E_SUCCESS or not doc_desc) {
+ JAMI_WARN("PUPnP: Error downloading device XML document from %s -> %s",
+ location.c_str(),
+ UpnpGetErrorMessage(upnp_err));
+ return std::make_unique<IGDInfo>(
+ IGDInfo {std::move(location), XMLDocument(nullptr, ixmlDocument_free)});
+ } else {
+ JAMI_DBG("PUPnP: Succeeded to download device XML document from %s", location.c_str());
+ return std::make_unique<IGDInfo>(IGDInfo {std::move(location), std::move(doc_desc)});
+ }
+ }));
}
void
@@ -813,26 +919,26 @@ PUPnP::processDiscoveryAdvertisementByebye(const std::string& cpDeviceId)
}
// Notify the listener.
- if (igd) {
+ if (observer_ and igd) {
observer_->onIgdUpdated(igd, UpnpIgdEvent::REMOVED);
}
}
void
-PUPnP::processDiscoverySubscriptionExpired(const std::string& eventSubUrl)
+PUPnP::processDiscoverySubscriptionExpired(Upnp_EventType event_type, const std::string& eventSubUrl)
{
std::lock_guard<std::mutex> lk(validIgdListMutex_);
for (auto& it : validIgdList_) {
if (auto igd = std::dynamic_pointer_cast<UPnPIGD>(it)) {
if (igd->getEventSubURL() == eventSubUrl) {
- JAMI_DBG("PUPnP: Received [%s] event for IGD [%s] %s. Will be renewed.",
- PUPnP::eventTypeToString(UPNP_EVENT_SUBSCRIPTION_EXPIRED),
+ JAMI_DBG("PUPnP: Received [%s] event for IGD [%s] %s. Request a new subscribe.",
+ PUPnP::eventTypeToString(event_type),
igd->getUID().c_str(),
igd->getLocalIp().toString().c_str());
std::lock_guard<std::mutex> lk1(ctrlptMutex_);
UpnpSubscribeAsync(ctrlptHandle_,
eventSubUrl.c_str(),
- SUBSCRIBE_TIMEOUT,
+ UPNP_INFINITE,
subEventCallback,
this);
break;
@@ -844,10 +950,6 @@ 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.
@@ -888,7 +990,6 @@ PUPnP::handleCtrlPtUPnPEvents(Upnp_EventType event_type, const void* event)
upnpThis->processDiscoveryAdvertisementByebye(deviceId);
}
});
-
break;
}
case UPNP_DISCOVERY_SEARCH_TIMEOUT: {
@@ -902,36 +1003,63 @@ PUPnP::handleCtrlPtUPnPEvents(Upnp_EventType event_type, const void* event)
// Nothing to do.
break;
}
- case UPNP_EVENT_AUTORENEWAL_FAILED: // Fall through. Treat failed autorenewal like an expired
- // subscription.
+ // Treat failed autorenewal like an expired subscription.
+ case UPNP_EVENT_AUTORENEWAL_FAILED:
case UPNP_EVENT_SUBSCRIPTION_EXPIRED: // This event will occur only if autorenewal is disabled.
{
+ JAMI_WARN("PUPnP: Received Subscription Event %s", eventTypeToString(event_type));
const UpnpEventSubscribe* es_event = (const UpnpEventSubscribe*) event;
+ if (es_event == nullptr) {
+ JAMI_WARN("PUPnP: Received Subscription Event with null pointer");
+ break;
+ }
std::string publisherUrl(UpnpEventSubscribe_get_PublisherUrl_cstr(es_event));
// Process the response on the main thread.
- runOnUpnpContextThread([w = weak(), publisherUrl = std::move(publisherUrl)] {
+ runOnUpnpContextThread([w = weak(), event_type, publisherUrl = std::move(publisherUrl)] {
if (auto upnpThis = w.lock()) {
- upnpThis->processDiscoverySubscriptionExpired(publisherUrl);
+ upnpThis->processDiscoverySubscriptionExpired(event_type, publisherUrl);
}
});
break;
}
- case UPNP_EVENT_SUBSCRIBE_COMPLETE: {
- break;
- }
+ case UPNP_EVENT_SUBSCRIBE_COMPLETE:
case UPNP_EVENT_UNSUBSCRIBE_COMPLETE: {
+ UpnpEventSubscribe* es_event = (UpnpEventSubscribe*) event;
+ if (es_event == nullptr) {
+ JAMI_WARN("PUPnP: Received Subscription Event with null pointer");
+ } else {
+ UpnpEventSubscribe_delete(es_event);
+ }
break;
}
case UPNP_CONTROL_ACTION_COMPLETE: {
const UpnpActionComplete* a_event = (const UpnpActionComplete*) event;
-
- if (UpnpActionComplete_get_ErrCode(a_event) == UPNP_E_SUCCESS) {
+ if (a_event == nullptr) {
+ JAMI_WARN("PUPnP: Received Action Complete Event with null pointer");
+ break;
+ }
+ auto res = UpnpActionComplete_get_ErrCode(a_event);
+ if (res != UPNP_E_SUCCESS and res != UPNP_E_TIMEDOUT) {
+ auto err = UpnpActionComplete_get_ErrCode(a_event);
+ JAMI_WARN("PUPnP: Received Action Complete error %i %s", err, UpnpGetErrorMessage(err));
+ } else {
auto actionRequest = UpnpActionComplete_get_ActionRequest(a_event);
// Abort if there is no action to process.
- if (actionRequest == nullptr)
+ if (actionRequest == nullptr) {
+ JAMI_WARN("PUPnP: Can't get the Action Request data from the event");
break;
+ }
+
+ auto actionResult = UpnpActionComplete_get_ActionResult(a_event);
+ if (actionResult != nullptr) {
+ ixmlDocument_free(actionResult);
+ } else {
+ JAMI_WARN("PUPnP: Action Result document not found");
+ }
+
auto upnpString = UpnpActionComplete_get_CtrlUrl(a_event);
+
std::string ctrlUrl {UpnpString_get_String(upnpString),
UpnpString_get_Length(upnpString)};
@@ -945,6 +1073,8 @@ PUPnP::handleCtrlPtUPnPEvents(Upnp_EventType event_type, const void* event)
std::string iPortStr(getFirstDocItem(actionRequest, "NewInternalPort"));
std::string portTypeStr(getFirstDocItem(actionRequest, "NewProtocol"));
+ ixmlDocument_free(actionRequest);
+
uint16_t ePort = ePortStr.empty() ? 0 : std::stoi(ePortStr);
uint16_t iPort = iPortStr.empty() ? 0 : std::stoi(iPortStr);
PortType portType = portTypeStr == "UDP" ? upnp::PortType::UDP
@@ -957,18 +1087,19 @@ PUPnP::handleCtrlPtUPnPEvents(Upnp_EventType event_type, const void* event)
ePort,
iPort,
portType] {
- if (auto upnpThis = w.lock()) {
- switch (ctrlAction) {
- case CtrlAction::ADD_PORT_MAPPING:
- upnpThis->processAddMapAction(ctrlUrl, ePort, iPort, portType);
- break;
- case CtrlAction::DELETE_PORT_MAPPING:
- upnpThis->processRemoveMapAction(ctrlUrl, ePort, iPort, portType);
- break;
- default:
- // All other control actions are ignored.
- break;
- }
+ auto upnpThis = w.lock();
+ if (not upnpThis)
+ return;
+ switch (ctrlAction) {
+ case CtrlAction::ADD_PORT_MAPPING:
+ upnpThis->processAddMapAction(ctrlUrl, ePort, iPort, portType);
+ break;
+ case CtrlAction::DELETE_PORT_MAPPING:
+ upnpThis->processRemoveMapAction(ctrlUrl, ePort, iPort, portType);
+ break;
+ default:
+ // All other control actions are ignored.
+ break;
}
});
}
@@ -994,16 +1125,15 @@ PUPnP::subEventCallback(Upnp_EventType event_type, const void* event, void* user
}
int
-PUPnP::handleSubscriptionUPnPEvent(Upnp_EventType /*unused*/, const void* event)
+PUPnP::handleSubscriptionUPnPEvent(Upnp_EventType event_type, const void* event)
{
- if (event == nullptr) {
+ UpnpEventSubscribe* es_event = static_cast<UpnpEventSubscribe*>(const_cast<void*>(event));
+
+ if (es_event == nullptr) {
JAMI_ERR("PUPnP: Unexpected null pointer!");
return UPNP_E_INVALID_ARGUMENT;
}
-
- const UpnpEventSubscribe* es_event = (const UpnpEventSubscribe*) event;
std::string publisherUrl(UpnpEventSubscribe_get_PublisherUrl_cstr(es_event));
-
int upnp_err = UpnpEventSubscribe_get_ErrCode(es_event);
if (upnp_err != UPNP_E_SUCCESS) {
JAMI_WARN("PUPnP: Subscription error %s from %s",
@@ -1144,9 +1274,9 @@ PUPnP::actionIsIgdConnected(const UPnPIGD& igd)
return false;
// Action and response pointers.
- XMLDocument action(nullptr, ixmlDocument_free); // Action pointer.
- XMLDocument response(nullptr, ixmlDocument_free); // Response pointer.
+ XMLDocument action(nullptr, ixmlDocument_free); // Action pointer.
IXML_Document* action_container_ptr = nullptr;
+ XMLDocument response(nullptr, ixmlDocument_free); // Response pointer.
IXML_Document* response_container_ptr = nullptr;
// Set action name.
@@ -1156,11 +1286,12 @@ PUPnP::actionIsIgdConnected(const UPnPIGD& igd)
igd.getServiceType().c_str(),
0,
nullptr);
- if (not action_container_ptr) {
+ action.reset(action_container_ptr);
+
+ if (not action) {
JAMI_WARN("PUPnP: Failed to make GetStatusInfo action");
return false;
}
- action.reset(action_container_ptr);
int upnp_err = UpnpSendAction(ctrlptHandle_,
igd.getControlURL().c_str(),
@@ -1168,11 +1299,12 @@ PUPnP::actionIsIgdConnected(const UPnPIGD& igd)
nullptr,
action.get(),
&response_container_ptr);
- if (upnp_err != UPNP_E_SUCCESS) {
+ response.reset(response_container_ptr);
+
+ if (not response or upnp_err != UPNP_E_SUCCESS) {
JAMI_WARN("PUPnP: Failed to send GetStatusInfo action -> %s", UpnpGetErrorMessage(upnp_err));
return false;
}
- response.reset(response_container_ptr);
if (errorOnResponse(response.get())) {
JAMI_WARN("PUPnP: Failed to get GetStatusInfo from %s -> %d: %s",
@@ -1207,11 +1339,12 @@ PUPnP::actionGetExternalIP(const UPnPIGD& igd)
IXML_Document* action_container_ptr = nullptr;
action_container_ptr = UpnpMakeAction(action_name, igd.getServiceType().c_str(), 0, nullptr);
- if (not action_container_ptr) {
+ action.reset(action_container_ptr);
+
+ if (not action) {
JAMI_WARN("PUPnP: Failed to make GetExternalIPAddress action");
return {};
}
- action.reset(action_container_ptr);
IXML_Document* response_container_ptr = nullptr;
int upnp_err = UpnpSendAction(ctrlptHandle_,
@@ -1220,12 +1353,13 @@ PUPnP::actionGetExternalIP(const UPnPIGD& igd)
nullptr,
action.get(),
&response_container_ptr);
- if (upnp_err != UPNP_E_SUCCESS) {
+ response.reset(response_container_ptr);
+
+ if (not response or upnp_err != UPNP_E_SUCCESS) {
JAMI_WARN("PUPnP: Failed to send GetExternalIPAddress action -> %s",
UpnpGetErrorMessage(upnp_err));
return {};
}
- response.reset(response_container_ptr);
if (errorOnResponse(response.get())) {
JAMI_WARN("PUPnP: Failed to get GetExternalIPAddress from %s -> %d: %s",
@@ -1255,9 +1389,10 @@ PUPnP::getMappingsListByDescr(const std::shared_ptr<IGD>& igd, const std::string
for (int entry_idx = 0;; entry_idx++) {
std::unique_ptr<IXML_Document, decltype(ixmlDocument_free)&>
action(nullptr, ixmlDocument_free); // Action pointer.
+ IXML_Document* action_container_ptr = nullptr;
+
std::unique_ptr<IXML_Document, decltype(ixmlDocument_free)&>
response(nullptr, ixmlDocument_free); // Response pointer.
- IXML_Document* action_container_ptr = nullptr;
IXML_Document* response_container_ptr = nullptr;
UpnpAddToAction(&action_container_ptr,
@@ -1267,6 +1402,11 @@ PUPnP::getMappingsListByDescr(const std::shared_ptr<IGD>& igd, const std::string
std::to_string(entry_idx).c_str());
action.reset(action_container_ptr);
+ if (not action) {
+ JAMI_WARN("PUPnP: Failed to add NewPortMappingIndex action");
+ break;
+ }
+
int upnp_err = UpnpSendAction(ctrlptHandle_,
upnpIgd->getControlURL().c_str(),
upnpIgd->getServiceType().c_str(),
@@ -1274,6 +1414,7 @@ PUPnP::getMappingsListByDescr(const std::shared_ptr<IGD>& igd, const std::string
action.get(),
&response_container_ptr);
response.reset(response_container_ptr);
+
if (not response) {
// No existing mapping. Abort silently.
break;
@@ -1359,161 +1500,107 @@ PUPnP::deleteMappingsByDescription(const std::shared_ptr<IGD>& igd, const std::s
}
bool
-PUPnP::actionDeletePortMapping(const UPnPIGD& igd,
- const std::string& port_external,
- const std::string& protocol)
+PUPnP::actionAddPortMapping(const std::shared_ptr<UPnPIGD>& igd, const Mapping& mapping)
{
if (not clientRegistered_)
return false;
// Action and response pointers.
- std::unique_ptr<IXML_Document, decltype(ixmlDocument_free)&>
- action(nullptr, ixmlDocument_free); // Action pointer.
- std::unique_ptr<IXML_Document, decltype(ixmlDocument_free)&>
- response(nullptr, ixmlDocument_free); // Response pointer.
- IXML_Document* action_container_ptr = nullptr;
-
- // Set action name.
- std::string action_name {"DeletePortMapping"};
-
- // Set action sequence.
- UpnpAddToAction(&action_container_ptr,
- action_name.c_str(),
- igd.getServiceType().c_str(),
- "NewRemoteHost",
- "");
- UpnpAddToAction(&action_container_ptr,
- action_name.c_str(),
- igd.getServiceType().c_str(),
- "NewExternalPort",
- port_external.c_str());
- UpnpAddToAction(&action_container_ptr,
- action_name.c_str(),
- igd.getServiceType().c_str(),
- "NewProtocol",
- protocol.c_str());
-
- action.reset(action_container_ptr);
- int upnp_err = UpnpSendActionAsync(ctrlptHandle_,
- igd.getControlURL().c_str(),
- igd.getServiceType().c_str(),
- nullptr,
- action.get(),
- ctrlPtCallback,
- this);
-
- if (upnp_err != UPNP_E_SUCCESS) {
- JAMI_WARN("PUPnP: Failed to send %s request from: %s, %d: %s",
- action_name.c_str(),
- igd.getServiceType().c_str(),
- upnp_err,
- UpnpGetErrorMessage(upnp_err));
- return false;
- }
-
- JAMI_DBG("PUPnP: Successfully sent %s request to %s",
- action_name.c_str(),
- igd.getServiceType().c_str());
-
- return true;
-}
-
-bool
-PUPnP::actionAddPortMapping(const UPnPIGD& igd,
- const Mapping& mapping,
- UPnPProtocol::UpnpError& error_code)
-{
- if (not clientRegistered_)
- return false;
-
- error_code = UPnPProtocol::UpnpError::ERROR_OK;
-
- // Action and response pointers.
- XMLDocument action(nullptr, ixmlDocument_free); // Action pointer.
- XMLDocument response(nullptr, ixmlDocument_free); // Response pointer.
+ XMLDocument action(nullptr, ixmlDocument_free);
IXML_Document* action_container_ptr = nullptr;
+ XMLDocument response(nullptr, ixmlDocument_free);
IXML_Document* response_container_ptr = nullptr;
- // Set action name.
- std::string action_name {"AddPortMapping"};
-
// Set action sequence.
UpnpAddToAction(&action_container_ptr,
- action_name.c_str(),
- igd.getServiceType().c_str(),
+ ACTION_ADD_PORT_MAPPING,
+ igd->getServiceType().c_str(),
"NewRemoteHost",
"");
UpnpAddToAction(&action_container_ptr,
- action_name.c_str(),
- igd.getServiceType().c_str(),
+ ACTION_ADD_PORT_MAPPING,
+ igd->getServiceType().c_str(),
"NewExternalPort",
mapping.getExternalPortStr().c_str());
UpnpAddToAction(&action_container_ptr,
- action_name.c_str(),
- igd.getServiceType().c_str(),
+ ACTION_ADD_PORT_MAPPING,
+ igd->getServiceType().c_str(),
"NewProtocol",
mapping.getTypeStr());
UpnpAddToAction(&action_container_ptr,
- action_name.c_str(),
- igd.getServiceType().c_str(),
+ ACTION_ADD_PORT_MAPPING,
+ igd->getServiceType().c_str(),
"NewInternalPort",
mapping.getInternalPortStr().c_str());
UpnpAddToAction(&action_container_ptr,
- action_name.c_str(),
- igd.getServiceType().c_str(),
+ ACTION_ADD_PORT_MAPPING,
+ igd->getServiceType().c_str(),
"NewInternalClient",
getHostAddress().toString().c_str());
UpnpAddToAction(&action_container_ptr,
- action_name.c_str(),
- igd.getServiceType().c_str(),
+ ACTION_ADD_PORT_MAPPING,
+ igd->getServiceType().c_str(),
"NewEnabled",
"1");
UpnpAddToAction(&action_container_ptr,
- action_name.c_str(),
- igd.getServiceType().c_str(),
+ ACTION_ADD_PORT_MAPPING,
+ igd->getServiceType().c_str(),
"NewPortMappingDescription",
mapping.toString().c_str());
UpnpAddToAction(&action_container_ptr,
- action_name.c_str(),
- igd.getServiceType().c_str(),
+ ACTION_ADD_PORT_MAPPING,
+ igd->getServiceType().c_str(),
"NewLeaseDuration",
"0");
action.reset(action_container_ptr);
int upnp_err = UpnpSendAction(ctrlptHandle_,
- igd.getControlURL().c_str(),
- igd.getServiceType().c_str(),
+ igd->getControlURL().c_str(),
+ igd->getServiceType().c_str(),
nullptr,
action.get(),
&response_container_ptr);
+ response.reset(response_container_ptr);
+
if (upnp_err != UPNP_E_SUCCESS) {
JAMI_WARN("PUPnP: Failed to send action %s from: %s, %d: %s",
- action_name.c_str(),
- igd.getServiceType().c_str(),
+ ACTION_ADD_PORT_MAPPING,
+ igd->getServiceType().c_str(),
upnp_err,
UpnpGetErrorMessage(upnp_err));
- error_code = UPnPProtocol::UpnpError::INVALID_ERR;
return false;
}
- if (not response_container_ptr) {
- JAMI_WARN("PUPnP: Failed to get response from %s", action_name.c_str());
+ if (not response) {
+ JAMI_WARN("PUPnP: Failed to get response from %s", ACTION_ADD_PORT_MAPPING);
return false;
}
- response.reset(response_container_ptr);
// Check if there is an error code.
std::string errorCode = getFirstDocItem(response.get(), "errorCode");
if (not errorCode.empty()) {
std::string errorDescription = getFirstDocItem(response.get(), "errorDescription");
JAMI_WARN("PUPnP: %s returned with error: %s: %s",
- action_name.c_str(),
+ ACTION_ADD_PORT_MAPPING,
errorCode.c_str(),
errorDescription.c_str());
- error_code = UPnPProtocol::UpnpError::INVALID_ERR;
return false;
}
+
+ JAMI_DBG("PUPnP: Sent request to open port %s", mapping.toString().c_str());
+
+ // Process the response on the main thread.
+ runOnUpnpContextThread([w = weak(), mapping, igd] {
+ auto upnpThis = w.lock();
+ if (not upnpThis)
+ return;
+ upnpThis->processAddMapAction(igd->getControlURL(),
+ mapping.getExternalPort(),
+ mapping.getInternalPort(),
+ mapping.getType());
+ });
+
return true;
}
@@ -1563,5 +1650,86 @@ PUPnP::actionDeletePortMappingAsync(const UPnPIGD& igd,
return true;
}
+bool
+PUPnP::actionDeletePortMapping(const std::shared_ptr<UPnPIGD>& igd, const Mapping& mapping)
+{
+ if (not clientRegistered_) {
+ return false;
+ }
+
+ // Action and response pointers.
+ XMLDocument action(nullptr, ixmlDocument_free);
+ IXML_Document* action_container_ptr = nullptr;
+ XMLDocument response(nullptr, ixmlDocument_free);
+ IXML_Document* response_container_ptr = nullptr;
+
+ // Set action sequence.
+ UpnpAddToAction(&action_container_ptr,
+ ACTION_DELETE_PORT_MAPPING,
+ igd->getServiceType().c_str(),
+ "NewRemoteHost",
+ "");
+ UpnpAddToAction(&action_container_ptr,
+ ACTION_DELETE_PORT_MAPPING,
+ igd->getServiceType().c_str(),
+ "NewExternalPort",
+ mapping.getExternalPortStr().c_str());
+ UpnpAddToAction(&action_container_ptr,
+ ACTION_DELETE_PORT_MAPPING,
+ igd->getServiceType().c_str(),
+ "NewProtocol",
+ mapping.getTypeStr());
+
+ action.reset(action_container_ptr);
+
+ int upnp_err = UpnpSendAction(ctrlptHandle_,
+ igd->getControlURL().c_str(),
+ igd->getServiceType().c_str(),
+ nullptr,
+ action.get(),
+ &response_container_ptr);
+ response.reset(response_container_ptr);
+
+ if (upnp_err != UPNP_E_SUCCESS) {
+ JAMI_WARN("PUPnP: Failed to send action %s from: %s, %d: %s",
+ ACTION_DELETE_PORT_MAPPING,
+ igd->getServiceType().c_str(),
+ upnp_err,
+ UpnpGetErrorMessage(upnp_err));
+ return false;
+ }
+
+ if (not response) {
+ JAMI_WARN("PUPnP: Failed to get response from %s", ACTION_DELETE_PORT_MAPPING);
+ return false;
+ }
+
+ // Check if there is an error code.
+ std::string errorCode = getFirstDocItem(response.get(), "errorCode");
+ if (not errorCode.empty()) {
+ std::string errorDescription = getFirstDocItem(response.get(), "errorDescription");
+ JAMI_WARN("PUPnP: %s returned with error: %s: %s",
+ ACTION_DELETE_PORT_MAPPING,
+ errorCode.c_str(),
+ errorDescription.c_str());
+ return false;
+ }
+
+ JAMI_DBG("PUPnP: Sent request to close port %s", mapping.toString().c_str());
+
+ // Process the response on the main thread.
+ runOnUpnpContextThread([w = weak(), mapping, igd] {
+ auto upnpThis = w.lock();
+ if (not upnpThis)
+ return;
+ upnpThis->processRemoveMapAction(igd->getControlURL(),
+ mapping.getExternalPort(),
+ mapping.getInternalPort(),
+ mapping.getType());
+ });
+
+ return true;
+}
+
} // namespace upnp
} // namespace jami
diff --git a/src/upnp/protocol/pupnp/pupnp.h b/src/upnp/protocol/pupnp/pupnp.h
index 3290bab6ae65e737c4fa1bdff2052bdce4a3d750..e490f5ffd6fdd19662f1773d6b2485c427f5e62a 100644
--- a/src/upnp/protocol/pupnp/pupnp.h
+++ b/src/upnp/protocol/pupnp/pupnp.h
@@ -69,7 +69,6 @@ constexpr static int CONFLICT_IN_MAPPING = 718;
// Timeout values (in seconds).
constexpr static unsigned int SEARCH_TIMEOUT {60};
-constexpr static unsigned int SUBSCRIBE_TIMEOUT {300};
// Max number of IGD search attempts before failure.
constexpr static unsigned int PUPNP_MAX_RESTART_SEARCH_RETRIES {5};
// Time-out between two successive IGD search.
@@ -98,15 +97,9 @@ public:
PUPnP();
~PUPnP();
- bool initUpnpLib();
- bool registerClient();
-
// Set the observer
void setObserver(UpnpMappingObserver* obs) override;
- // Start search for UPNP devices
- void searchForDevices();
-
// Returns the protocol type.
NatProtocolType getProtocol() const override { return NatProtocolType::PUPNP; }
@@ -142,9 +135,25 @@ public:
// Removes a mapping.
void requestMappingRemove(const Mapping& igdMapping) override;
+ void terminate() override;
+
private:
+ NON_COPYABLE(PUPnP);
+
ScheduledExecutor* getUpnContextScheduler() { return UpnpThreadUtil::getScheduler(); }
+ // Init lib-upnp
+ void initUpnpLib();
+
+ // Register the client
+ void registerClient();
+
+ // Start the internal thread.
+ void startPUPnP();
+
+ // Start search for UPNP devices
+ void searchForDevices();
+
// Return true if it has at least one valid IGD.
bool hasValidIgd() const;
@@ -168,12 +177,12 @@ private:
uint16_t iPort,
PortType portType);
- // Returns control point action callback based on xml node.
- CtrlAction getAction(const char* xmlNode);
-
// Validate IGD from the xml document received from the router.
bool validateIgd(const IGDInfo&);
+ // Returns control point action callback based on xml node.
+ static CtrlAction getAction(const char* xmlNode);
+
// Control point callback.
static int ctrlPtCallback(Upnp_EventType event_type, const void* event, void* user_data);
#if UPNP_VERSION < 10800
@@ -187,7 +196,8 @@ private:
const std::string& igdUrl,
const IpAddr& dstAddr);
void processDiscoveryAdvertisementByebye(const std::string& deviceId);
- void processDiscoverySubscriptionExpired(const std::string& eventSubUrl);
+ void processDiscoverySubscriptionExpired(Upnp_EventType event_type,
+ const std::string& eventSubUrl);
// Callback event handler function for the UPnP client (control point).
int handleCtrlPtUPnPEvents(Upnp_EventType event_type, const void* event);
@@ -202,7 +212,7 @@ private:
#endif
// Callback subscription event function for handling subscription request.
- int handleSubscriptionUPnPEvent(Upnp_EventType /*unused*/, const void* event);
+ int handleSubscriptionUPnPEvent(Upnp_EventType event_type, const void* event);
// Parses the IGD candidate.
std::unique_ptr<UPnPIGD> parseIgd(IXML_Document* doc, std::string locationUrl);
@@ -212,30 +222,24 @@ private:
bool actionIsIgdConnected(const UPnPIGD& igd);
IpAddr actionGetExternalIP(const UPnPIGD& igd);
- bool actionDeletePortMapping(const UPnPIGD& igd,
- const std::string& port_external,
- const std::string& protocol);
- bool actionAddPortMapping(const UPnPIGD& igd,
- const Mapping& mapping,
- UPnPProtocol::UpnpError& upnp_error);
- bool actionAddPortMappingAsync(const UPnPIGD& igd, const Mapping& mapping);
+ bool actionAddPortMapping(const std::shared_ptr<UPnPIGD>& igd, const Mapping& mapping);
+ bool actionAddPortMappingAsync(const std::shared_ptr<UPnPIGD>& igd, const Mapping& mapping);
bool actionDeletePortMappingAsync(const UPnPIGD& igd,
const std::string& port_external,
const std::string& protocol);
+ bool actionDeletePortMapping(const std::shared_ptr<UPnPIGD>& igd, const Mapping& mapping);
// Event type to string
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);
+ // Initialization status.
+ std::atomic_bool initialized_ {false};
+ // Client registration status.
+ std::atomic_bool clientRegistered_ {false};
std::condition_variable pupnpCv_ {}; // Condition variable for thread-safe signaling.
- std::atomic_bool pupnpRun_ {true}; // Variable to allow the thread to run.
+ std::atomic_bool pupnpRun_ {false}; // Variable to allow the thread to run.
std::thread pupnpThread_ {}; // PUPnP thread for non-blocking client registration.
std::shared_ptr<Task> searchForIgdTimer_ {};
unsigned int igdSearchCounter_ {0};
@@ -248,8 +252,7 @@ private:
dwnldlXmlList_; // List of futures for blocking xml download function calls.
std::list<std::future<pIGDInfo>> cancelXmlList_; // List of abandoned documents
- mutable std::mutex igdListMutex_; // Mutex used to protect IGD instances.
-
+ mutable std::mutex igdListMutex_; // Mutex used to protect IGD instances.
std::mutex ctrlptMutex_; // Mutex for client handle protection.
UpnpClient_Handle ctrlptHandle_ {-1}; // Control point handle.
diff --git a/src/upnp/protocol/upnp_protocol.h b/src/upnp/protocol/upnp_protocol.h
index eed6cf00f6ed936e18160efa5291896acc44f90c..ce229b2f13c195268f5c13a16480e8fc9807f433 100644
--- a/src/upnp/protocol/upnp_protocol.h
+++ b/src/upnp/protocol/upnp_protocol.h
@@ -124,10 +124,13 @@ public:
// Get the host (local) address.
const IpAddr& getHostAddress() const { return hostAddress_; }
+ // Terminate
+ virtual void terminate() = 0;
+
protected:
// The host (local) address. Must be fully set before making any request.
IpAddr hostAddress_ {};
- UpnpMappingObserver* observer_;
+ UpnpMappingObserver* observer_ {nullptr};
};
} // namespace upnp
diff --git a/src/upnp/upnp_context.cpp b/src/upnp/upnp_context.cpp
index e52f86bc1024df8b837c090da3519fd16d077f0c..f4031a92dbdcf51718b3c2fb6d5097c6c1d4f054 100644
--- a/src/upnp/upnp_context.cpp
+++ b/src/upnp/upnp_context.cpp
@@ -35,14 +35,6 @@ constexpr static uint16_t UPNP_TCP_PORT_MAX {UPNP_TCP_PORT_MIN + 5000};
constexpr static uint16_t UPNP_UDP_PORT_MIN {20000};
constexpr static uint16_t UPNP_UDP_PORT_MAX {UPNP_UDP_PORT_MIN + 5000};
-std::shared_ptr<UPnPContext>
-UPnPContext::getUPnPContext()
-{
- // This is the unique shared instance (singleton) of UPnPContext class.
- static auto context = std::make_shared<UPnPContext>();
- return context;
-}
-
UPnPContext::UPnPContext()
{
JAMI_DBG("Creating UPnPContext instance [%p]", this);
@@ -57,15 +49,39 @@ UPnPContext::UPnPContext()
}
}
-UPnPContext::~UPnPContext()
+std::shared_ptr<UPnPContext>
+UPnPContext::getUPnPContext()
{
+ // This is the unique shared instance (singleton) of UPnPContext class.
+ static auto context = std::make_shared<UPnPContext>();
+ return context;
+}
+
+void
+UPnPContext::shutdown()
+{
+ if (not isValidThread()) {
+ runOnUpnpContextThread([this] { shutdown(); });
+ return;
+ }
+
+ JAMI_DBG("Shutdown UPnPContext instance [%p]", this);
+
stopUpnp(true);
+ for (auto const& [_, proto] : protocolList_)
+ proto->terminate();
+
std::lock_guard<std::mutex> lock(mappingMutex_);
mappingList_->clear();
- mappingListUpdateTimer_->cancel();
+ if (mappingListUpdateTimer_)
+ mappingListUpdateTimer_->cancel();
controllerList_.clear();
+ protocolList_.clear();
+}
+UPnPContext::~UPnPContext()
+{
JAMI_DBG("UPnPContext instance [%p] destroyed", this);
}
@@ -113,7 +129,7 @@ UPnPContext::stopUpnp(bool forceRelease)
return;
}
- JAMI_DBG("Stoping UPNP context");
+ JAMI_DBG("Stopping UPNP context");
// Clear all current mappings if any.
@@ -186,8 +202,6 @@ UPnPContext::connectivityChanged()
return;
}
- CHECK_VALID_THREAD();
-
if (controllerList_.empty())
return;
@@ -297,8 +311,6 @@ UPnPContext::releaseMapping(const Mapping& map)
return;
}
- CHECK_VALID_THREAD();
-
auto mapPtr = getMappingWithKey(map.getMapKey());
if (not mapPtr) {
@@ -327,11 +339,11 @@ UPnPContext::registerController(void* controller)
auto ret = controllerList_.emplace(controller);
if (not ret.second) {
- JAMI_WARN("Controller %p is already registered", this);
+ JAMI_WARN("Controller %p is already registered", controller);
return;
}
- JAMI_DBG("Successfully registered controller %p", this);
+ JAMI_DBG("Successfully registered controller %p", controller);
if (not started_)
startUpnp();
}
@@ -345,13 +357,14 @@ UPnPContext::unregisterController(void* controller)
}
if (controllerList_.erase(controller) == 1) {
- JAMI_DBG("Successfully unregistered controller %p", this);
+ JAMI_DBG("Successfully unregistered controller %p", controller);
} else {
- JAMI_ERR("Trying to unregister an unknown controller %p", this);
+ JAMI_DBG("Controller %p was already removed", controller);
}
- if (controllerList_.empty())
+ if (controllerList_.empty()) {
stopUpnp();
+ }
}
uint16_t
@@ -385,8 +398,6 @@ UPnPContext::requestMapping(const Mapping::sharedPtr_t& map)
return;
}
- CHECK_VALID_THREAD();
-
auto const& igd = getPreferredIgd();
// We must have at least a valid IGD pointer if we get here.
// Not this method is called only if there were a valid IGD, however,
@@ -836,8 +847,6 @@ UPnPContext::onIgdUpdated(const std::shared_ptr<IGD>& igd, UpnpIgdEvent event)
return;
}
- CHECK_VALID_THREAD();
-
char const* IgdState = event == UpnpIgdEvent::ADDED
? "ADDED"
: event == UpnpIgdEvent::REMOVED ? "REMOVED" : "INVALID";
@@ -1023,8 +1032,6 @@ UPnPContext::deleteAllMappings(PortType type)
return;
}
- CHECK_VALID_THREAD();
-
std::lock_guard<std::mutex> lock(mappingMutex_);
auto& mappingList = getMappingList(type);
@@ -1044,8 +1051,6 @@ UPnPContext::onMappingRemoved(const std::shared_ptr<IGD>& igd, const Mapping& ma
return;
}
- CHECK_VALID_THREAD();
-
auto map = getMappingWithKey(mapRes.getMapKey());
// Notify the listener.
if (map and map->getNotifyCallback())
diff --git a/src/upnp/upnp_context.h b/src/upnp/upnp_context.h
index d6879ff8c9930fd56a147cc315fa7b1737127bdb..6ce476a03e0fcf399cf5e2aa7bad2beed953d69d 100644
--- a/src/upnp/upnp_context.h
+++ b/src/upnp/upnp_context.h
@@ -90,9 +90,12 @@ public:
UPnPContext();
~UPnPContext();
- // Retrieve the UPnPContext singleton
+ // Retrieve the UPnPContext singleton.
static std::shared_ptr<UPnPContext> getUPnPContext();
+ // Terminate the instance.
+ void shutdown();
+
// Set the known public address
void setPublicAddress(const IpAddr& addr);