upnp: debounce connectivityChanged
Change-Id: I6c7c73801febe3dc017ee58dfeff4d4ad692a3c8
diff --git a/include/upnp/upnp_context.h b/include/upnp/upnp_context.h
index 3f24bca..11f7e29 100644
--- a/include/upnp/upnp_context.h
+++ b/include/upnp/upnp_context.h
@@ -277,6 +277,11 @@
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 @@
IpAddr knownPublicAddress_ {};
// Set of registered controllers
+ std::mutex mutable controllerMutex_;
std::set<void*> controllerList_;
// Map of available protocols.
@@ -300,6 +306,7 @@
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 @@
// 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 9d79f49..af7c2db 100644
--- a/src/upnp/upnp_context.cpp
+++ b/src/upnp/upnp_context.cpp
@@ -44,7 +44,10 @@
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 @@
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 @@
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 @@
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 @@
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 @@
}
if (controllerList_.empty()) {
+ lock.unlock();
stopUpnp();
}
}