upnp: debounce connectivityChanged

Change-Id: I6c7c73801febe3dc017ee58dfeff4d4ad692a3c8

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

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,11 +393,7 @@ 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());
 
         if (not mapPtr) {
@@ -405,6 +410,7 @@ UPnPContext::releaseMapping(const Mapping& map)
         // Remove it.
         requestRemoveMapping(mapPtr);
         unregisterMapping(mapPtr);
+    });
 }
 
 void
@@ -416,18 +422,12 @@ 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;
         }
+    }
 
     if (logger_) logger_->debug("Successfully registered controller {}", fmt::ptr(controller));
     if (not started_)
@@ -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();
     }
 }