diff --git a/include/upnp/upnp_context.h b/include/upnp/upnp_context.h index 3f24bca3b2ac85545cca65cc0413e5c19aa2dab5..11f7e29f6cb8afb925cf8edf52d89aab628496af 100644 --- a/include/upnp/upnp_context.h +++ b/include/upnp/upnp_context.h @@ -277,6 +277,11 @@ private: UPnPContext& operator=(UPnPContext&&) = delete; UPnPContext& operator=(const UPnPContext&) = delete; + void _connectivityChanged(const asio::error_code& ec); + + // Thread (io_context), destroyed last + std::unique_ptr<std::thread> ioContextRunner_ {}; + bool started_ {false}; // The known public address. The external addresses returned by @@ -284,6 +289,7 @@ private: IpAddr knownPublicAddress_ {}; // Set of registered controllers + std::mutex mutable controllerMutex_; std::set<void*> controllerList_; // Map of available protocols. @@ -300,6 +306,7 @@ private: std::shared_ptr<asio::io_context> ctx; std::shared_ptr<dht::log::Logger> logger_; asio::steady_timer mappingListUpdateTimer_; + asio::steady_timer connectivityChangedTimer_; // Current preferred IGD. Can be null if there is no valid IGD. std::shared_ptr<IGD> preferredIgd_; @@ -313,9 +320,6 @@ private: // Shutdown synchronization bool shutdownComplete_ {false}; - - // Thread - std::unique_ptr<std::thread> ioContextRunner_; }; } // namespace upnp diff --git a/src/upnp/upnp_context.cpp b/src/upnp/upnp_context.cpp index 9d79f49520dd782f375895a87cc00a5eb6664b65..af7c2db366d63a6f995d2bac88ca735730db27e4 100644 --- a/src/upnp/upnp_context.cpp +++ b/src/upnp/upnp_context.cpp @@ -44,7 +44,10 @@ constexpr static uint16_t UPNP_UDP_PORT_MIN {20000}; constexpr static uint16_t UPNP_UDP_PORT_MAX {UPNP_UDP_PORT_MIN + 5000}; UPnPContext::UPnPContext(const std::shared_ptr<asio::io_context>& ioContext, const std::shared_ptr<dht::log::Logger>& logger) - : ctx(createIoContext(ioContext, logger)), mappingListUpdateTimer_(*ioContext), logger_(logger) + : ctx(createIoContext(ioContext, logger)) + , mappingListUpdateTimer_(*ioContext) + , connectivityChangedTimer_(*ioContext) + , logger_(logger) { if (logger_) logger_->debug("Creating UPnPContext instance [{}]", fmt::ptr(this)); @@ -238,10 +241,16 @@ UPnPContext::generateRandomPort(PortType type, bool mustBeEven) void UPnPContext::connectivityChanged() { - /*if (not isValidThread()) { - ctx->post([this] { connectivityChanged(); }); + // Debounce the connectivity change notification. + connectivityChangedTimer_.expires_after(std::chrono::milliseconds(50)); + connectivityChangedTimer_.async_wait(std::bind(&UPnPContext::_connectivityChanged, this, std::placeholders::_1)); +} + +void +UPnPContext::_connectivityChanged(const asio::error_code& ec) +{ + if (ec == asio::error::operation_aborted) return; - }*/ auto hostAddr = ip_utils::getLocalAddr(AF_INET); @@ -384,27 +393,24 @@ UPnPContext::reserveMapping(Mapping& requestedMap) void UPnPContext::releaseMapping(const Mapping& map) { - /*if (not isValidThread()) { - ctx->post([this, map] { releaseMapping(map); }); - return; - }*/ + ctx->dispatch([this, map] { + auto mapPtr = getMappingWithKey(map.getMapKey()); - auto mapPtr = getMappingWithKey(map.getMapKey()); - - if (not mapPtr) { - // Might happen if the mapping failed or was never granted. - if (logger_) logger_->debug("Mapping {} does not exist or was already removed", map.toString()); - return; - } + if (not mapPtr) { + // Might happen if the mapping failed or was never granted. + if (logger_) logger_->debug("Mapping {} does not exist or was already removed", map.toString()); + return; + } - if (mapPtr->isAvailable()) { - if (logger_) logger_->warn("Trying to release an unused mapping {}", mapPtr->toString()); - return; - } + if (mapPtr->isAvailable()) { + if (logger_) logger_->warn("Trying to release an unused mapping {}", mapPtr->toString()); + return; + } - // Remove it. - requestRemoveMapping(mapPtr); - unregisterMapping(mapPtr); + // Remove it. + requestRemoveMapping(mapPtr); + unregisterMapping(mapPtr); + }); } void @@ -416,17 +422,11 @@ UPnPContext::registerController(void* controller) if (logger_) logger_->warn("UPnPContext already shut down"); return; } - } - - /*if (not isValidThread()) { - ctx->post([this, controller] { registerController(controller); }); - return; - }*/ - - auto ret = controllerList_.emplace(controller); - if (not ret.second) { - if (logger_) logger_->warn("Controller {} is already registered", fmt::ptr(controller)); - return; + auto ret = controllerList_.emplace(controller); + if (not ret.second) { + if (logger_) logger_->warn("Controller {} is already registered", fmt::ptr(controller)); + return; + } } if (logger_) logger_->debug("Successfully registered controller {}", fmt::ptr(controller)); @@ -437,11 +437,7 @@ UPnPContext::registerController(void* controller) void UPnPContext::unregisterController(void* controller) { - /*if (not isValidThread()) { - ctx->post([this, controller] { unregisterController(controller); }); - return; - }*/ - + std::unique_lock<std::mutex> lock(mappingMutex_); if (controllerList_.erase(controller) == 1) { if (logger_) logger_->debug("Successfully unregistered controller {}", fmt::ptr(controller)); } else { @@ -449,6 +445,7 @@ UPnPContext::unregisterController(void* controller) } if (controllerList_.empty()) { + lock.unlock(); stopUpnp(); } }