commit 5a9e80100fbea2f0e13cc08b4455accc1124e925
author François-Simon Fauteux-Chapleau <francois-simon.fauteux-chapleau@savoirfairelinux.com> Tue Jun 04 11:39:17 2024 -0400
committer François-Simon Fauteux-Chapleau <francois-simon.fauteux-chapleau@savoirfairelinux.com> Tue Jun 04 13:52:54 2024 -0400
tree 6d7897b8c877855d76129b6235abf8699ed6157b
parent 826f0ba4ef26479b7bc2886e4280b1d430b215f8

upnp: only update preferredIgd_ when needed

This patch fixes a bug in the onIgdUpdated function, which currently
resets the UPnPContext's preferred IGD every time it is called, even if
the IGD was still valid. This can cause port mappings to be needlessly
deleted and recreated.

Change-Id: I5150e38a8d5d37f2823433e39aa69c1837cedc13

src/upnp/upnp_context.cpp

diff --git a/src/upnp/upnp_context.cpp b/src/upnp/upnp_context.cpp
index 7ea3d83..e161a43 100644
--- a/src/upnp/upnp_context.cpp
+++ b/src/upnp/upnp_context.cpp
@@ -595,8 +595,6 @@
 void
 UPnPContext::updatePreferredIgd()
 {
-    //CHECK_VALID_THREAD();
-
     if (preferredIgd_ and preferredIgd_->isValid())
         return;
 
@@ -645,11 +643,6 @@
         return;
     }
 
-    //CHECK_VALID_THREAD();
-
-    // Update the preferred IGD.
-    updatePreferredIgd();
-
     mappingListUpdateTimer_.cancel();
 
     // Skip if no controller registered.
@@ -954,9 +947,6 @@
 {
     assert(igd);
 
-    // Reset to start search for a new best IGD.
-    preferredIgd_.reset();
-
     char const* IgdState = event == UpnpIgdEvent::ADDED     ? "ADDED"
                            : event == UpnpIgdEvent::REMOVED ? "REMOVED"
                                                             : "INVALID";
@@ -989,7 +979,7 @@
                   knownPublicAddress_.toString());
     }
 
-    // The IGD was removed or is invalid.
+    // Update the IGD list.
     if (event == UpnpIgdEvent::REMOVED or event == UpnpIgdEvent::INVALID_STATE) {
         if (logger_) logger_->warn("State of IGD [{} {}] [{}] changed to [{}]. Pruning the mapping list",
                   igd->getUID(),
@@ -1001,11 +991,7 @@
 
         std::lock_guard lock(mappingMutex_);
         validIgdList_.erase(igd);
-        return;
-    }
-
-    // Update the IGD list.
-    {
+    } else {
         std::lock_guard lock(mappingMutex_);
         auto ret = validIgdList_.emplace(igd);
         if (ret.second) {
@@ -1021,6 +1007,8 @@
         }
     }
 
+    updatePreferredIgd();
+
     // Update the provisionned mappings.
     updateMappingList(false);
 }