diff --git a/src/upnp/protocol/natpmp/nat_pmp.cpp b/src/upnp/protocol/natpmp/nat_pmp.cpp index 68cbf44b93c283256217f90ccfde8344df2427e3..6562506628beab7a6f82ab90b289f77d00438410 100644 --- a/src/upnp/protocol/natpmp/nat_pmp.cpp +++ b/src/upnp/protocol/natpmp/nat_pmp.cpp @@ -121,17 +121,6 @@ NatPmp::initNatPmp() }; } -void -NatPmp::waitForShutdown() -{ - std::unique_lock<std::mutex> lk(natpmpMutex_); - if (shutdownCv_.wait_for(lk, std::chrono::seconds(10), [this] { return shutdownComplete_; })) { - JAMI_DBG("NAT-PMP: Shutdown completed"); - } else { - JAMI_ERR("NAT-PMP: Shutdown timed-out"); - } -} - void NatPmp::setObserver(UpnpMappingObserver* obs) { @@ -150,27 +139,35 @@ NatPmp::setObserver(UpnpMappingObserver* obs) } void -NatPmp::terminate() +NatPmp::terminate(std::condition_variable& cv) { - if (not isValidThread()) { - runOnNatPmpQueue([w = weak()] { - if (auto pmpThis = w.lock()) { - pmpThis->terminate(); - } - }); - waitForShutdown(); - return; - } - initialized_ = false; observer_ = nullptr; { std::lock_guard<std::mutex> lock(natpmpMutex_); shutdownComplete_ = true; + cv.notify_one(); } +} + +void +NatPmp::terminate() +{ + std::unique_lock<std::mutex> lk(natpmpMutex_); + std::condition_variable cv {}; - shutdownCv_.notify_one(); + runOnNatPmpQueue([w = weak(), &cv = cv] { + if (auto pmpThis = w.lock()) { + pmpThis->terminate(cv); + } + }); + + if (cv.wait_for(lk, std::chrono::seconds(10), [this] { return shutdownComplete_; })) { + JAMI_DBG("NAT-PMP: Shutdown completed"); + } else { + JAMI_ERR("NAT-PMP: Shutdown timed-out"); + } } const IpAddr diff --git a/src/upnp/protocol/natpmp/nat_pmp.h b/src/upnp/protocol/natpmp/nat_pmp.h index b3f9e33536f213856331fa5cd808375114285007..addd15284978a3e3f56f2f36f92d2df45d7e63b9 100644 --- a/src/upnp/protocol/natpmp/nat_pmp.h +++ b/src/upnp/protocol/natpmp/nat_pmp.h @@ -118,8 +118,9 @@ private: // Helpers to run tasks on UPNP context execution queue. ScheduledExecutor* getUpnContextScheduler() { return UpnpThreadUtil::getScheduler(); } + void terminate(std::condition_variable& cv); + void initNatPmp(); - void waitForShutdown(); void getIgdPublicAddress(); void removeAllMappings(); int readResponse(natpmp_t& handle, natpmpresp_t& response); @@ -170,7 +171,6 @@ private: mutable std::mutex natpmpMutex_; // Shutdown synchronization - std::condition_variable shutdownCv_ {}; bool shutdownComplete_ {false}; }; diff --git a/src/upnp/protocol/pupnp/pupnp.cpp b/src/upnp/protocol/pupnp/pupnp.cpp index 481d7c89c65458b9d164a7765325a77ac33a5b1b..c43d0311c22058b99d53b942d60c3ac596cb1b47 100644 --- a/src/upnp/protocol/pupnp/pupnp.cpp +++ b/src/upnp/protocol/pupnp/pupnp.cpp @@ -166,19 +166,6 @@ PUPnP::initUpnpLib() initialized_ = true; } -void -PUPnP::waitForShutdown() -{ - std::unique_lock<std::mutex> lk(pupnpMutex_); - if (shutdownCv_.wait_for(lk, std::chrono::seconds(10), [this] { return shutdownComplete_; })) { - JAMI_DBG("PUPnP: Shutdown completed"); - } else { - JAMI_ERR("PUPnP: Shutdown timed-out"); - // Force stop if the shutdown take too much time. - shutdownComplete_ = true; - } -} - bool PUPnP::isRunning() const { @@ -228,18 +215,8 @@ PUPnP::getHostAddress() const } void -PUPnP::terminate() +PUPnP::terminate(std::condition_variable& cv) { - if (not isValidThread()) { - runOnPUPnPQueue([w = weak()] { - if (auto upnpThis = w.lock()) { - upnpThis->terminate(); - } - }); - waitForShutdown(); - return; - } - JAMI_DBG("PUPnP: Terminate instance %p", this); clientRegistered_ = false; @@ -262,9 +239,29 @@ PUPnP::terminate() std::lock_guard<std::mutex> lock(pupnpMutex_); validIgdList_.clear(); shutdownComplete_ = true; + cv.notify_one(); } +} + +void +PUPnP::terminate() +{ + std::unique_lock<std::mutex> lk(pupnpMutex_); + std::condition_variable cv {}; + + runOnPUPnPQueue([w = weak(), &cv = cv] { + if (auto upnpThis = w.lock()) { + upnpThis->terminate(cv); + } + }); - shutdownCv_.notify_one(); + if (cv.wait_for(lk, std::chrono::seconds(10), [this] { return shutdownComplete_; })) { + JAMI_DBG("PUPnP: Shutdown completed"); + } else { + JAMI_ERR("PUPnP: Shutdown timed-out"); + // Force stop if the shutdown take too much time. + shutdownComplete_ = true; + } } void diff --git a/src/upnp/protocol/pupnp/pupnp.h b/src/upnp/protocol/pupnp/pupnp.h index 7d0d3d054cfc7bf1025a656aaacd8acde7820bb8..a15a9d34a8d97c4019da04c40aa4f348411ad194 100644 --- a/src/upnp/protocol/pupnp/pupnp.h +++ b/src/upnp/protocol/pupnp/pupnp.h @@ -136,12 +136,11 @@ private: // Helper to run tasks on UPNP context execution queue. ScheduledExecutor* getUpnContextScheduler() { return UpnpThreadUtil::getScheduler(); } + void terminate(std::condition_variable& cv); + // Init lib-upnp void initUpnpLib(); - // Block until shutdown is complete or time-out. - void waitForShutdown(); - // Return true if running. bool isRunning() const; @@ -269,7 +268,6 @@ private: mutable std::mutex pupnpMutex_; // Shutdown synchronization - std::condition_variable shutdownCv_ {}; bool shutdownComplete_ {false}; }; diff --git a/src/upnp/upnp_context.cpp b/src/upnp/upnp_context.cpp index 184d2057d5026fbf42d1c90b8de175a308dfd701..a447e2d75464e847a1f01830aa324f2b1c0d5657 100644 --- a/src/upnp/upnp_context.cpp +++ b/src/upnp/upnp_context.cpp @@ -57,20 +57,15 @@ UPnPContext::getUPnPContext() } void -UPnPContext::shutdown() +UPnPContext::shutdown(std::condition_variable& cv) { - if (not isValidThread()) { - runOnUpnpContextQueue([this] { shutdown(); }); - waitForShutdown(); - return; - } - JAMI_DBG("Shutdown UPnPContext instance [%p]", this); stopUpnp(true); - for (auto const& [_, proto] : protocolList_) + for (auto const& [_, proto] : protocolList_) { proto->terminate(); + } { std::lock_guard<std::mutex> lock(mappingMutex_); @@ -80,17 +75,23 @@ UPnPContext::shutdown() controllerList_.clear(); protocolList_.clear(); shutdownComplete_ = true; + cv.notify_one(); } - - shutdownCv_.notify_one(); } void -UPnPContext::waitForShutdown() +UPnPContext::shutdown() { - JAMI_DBG("Waiting for shutdown ..."); std::unique_lock<std::mutex> lk(mappingMutex_); - if (shutdownCv_.wait_for(lk, std::chrono::seconds(30), [this] { return shutdownComplete_; })) { + std::condition_variable cv; + + runOnUpnpContextQueue([&, this] { + shutdown(cv); + }); + + JAMI_DBG("Waiting for shutdown ..."); + + if (cv.wait_for(lk, std::chrono::seconds(30), [this] { return shutdownComplete_; })) { JAMI_DBG("Shutdown completed"); } else { JAMI_ERR("Shutdown timed-out"); diff --git a/src/upnp/upnp_context.h b/src/upnp/upnp_context.h index 7718fdcfea534a8810bebc89af86082b85c4466d..dea96d2fc70b2325a2bd707a2061f58fc7a93092 100644 --- a/src/upnp/upnp_context.h +++ b/src/upnp/upnp_context.h @@ -127,9 +127,6 @@ private: // Initialization void init(); - // Wait for Shutdown - void waitForShutdown(); - /** * @brief start the search for IGDs activate the mapping * list update. @@ -146,6 +143,8 @@ private: */ void stopUpnp(bool forceRelease = false); + void shutdown(std::condition_variable& cv); + // Create and register a new mapping. Mapping::sharedPtr_t registerMapping(Mapping& map); @@ -290,7 +289,6 @@ private: std::set<std::shared_ptr<IGD>> validIgdList_ {}; // Shutdown synchronization - std::condition_variable shutdownCv_ {}; bool shutdownComplete_ {false}; };