upnp/pupnp: request all mapping remove on IGD discovery

Send a request to delete all mappings created by previous
JAMI instances as soon as a new upnp IGD is discovered.
This is necessary with some implementations since they may
not respond well if the list of existing mappings is large.
Also make new requests for mappings with failing states.

Change-Id: I97308f1620128a63f111bcfc02122b472def5c85
Gitlab: #335

src/upnp/protocol/pupnp/pupnp.cpp

@@ -419,6 +419,9 @@ PUPnP::validateIgd(const IGDInfo& info)
         validIgdList_.emplace_back(igd_candidate);
     }
 
+    // Clear existing mappings
+    removeAllLocalMappings(igd_candidate);
+
     // Report to the listener.
     observer_->onIgdUpdated(igd_candidate, UpnpIgdEvent::ADDED);
 

src/upnp/upnp_context.cpp

@@ -506,6 +506,10 @@ UPnPContext::updateMappingList(bool async)
         return;
     }
 
+    // Make new requests for mappings that failed and have
+    // the auto-update option enabled.
+    processMappingWithAutoUpdate();
+
     PortType typeArray[2] = {PortType::TCP, PortType::UDP};
 
     for (auto idx : {0, 1}) {
@@ -729,6 +733,45 @@ UPnPContext::processPendingRequests(const std::shared_ptr<IGD>& igd)
     }
 }
 
+void
+UPnPContext::processMappingWithAutoUpdate()
+{
+    // This list holds the mappings to be requested. This is
+    // needed to avoid performing the requests while holding
+    // the lock.
+    std::list<Mapping::sharedPtr_t> requestsList;
+
+    // Populate the list of requests for mappings with auto-update enabled.
+    {
+        std::lock_guard<std::mutex> lock(mappingMutex_);
+        PortType typeArray[2] {PortType::TCP, PortType::UDP};
+
+        for (auto type : typeArray) {
+            auto& mappingList = getMappingList(type);
+            for (auto const& [_, map] : mappingList) {
+                if (map->getState() == MappingState::FAILED and map->getAutoUpdate()) {
+                    requestsList.emplace_back(map);
+                }
+            }
+        }
+    }
+
+    for (auto const& map : requestsList) {
+        // Request a new mapping if auto-update is enabled.
+        JAMI_DBG("Mapping %s has auto-update enabled, a new mapping will be requested",
+                 map->toString().c_str());
+
+        // Reserve a new mapping.
+        Mapping newMapping(*map);
+        newMapping.setExternalPort(0);
+        newMapping.setInternalPort(0);
+        registerMapping(newMapping);
+        // Release the old one.
+        map->setAvailable(true);
+        unregisterMapping(map);
+    }
+}
+
 void
 UPnPContext::onIgdUpdated(const std::shared_ptr<IGD>& igd, UpnpIgdEvent event)
 {
@@ -1178,26 +1221,6 @@ UPnPContext::updateMappingState(const Mapping::sharedPtr_t& map, MappingState ne
     // Notify the listener if set.
     if (notify and map->getNotifyCallback())
         map->getNotifyCallback()(map);
-
-    // Nothing more to do if there is no valid IGD.
-    if (not hasValidIGD())
-        return;
-
-    // On fail, request a new mapping if auto-update is enabled.
-    if (newState == MappingState::FAILED and map->getAutoUpdate()) {
-        JAMI_DBG("Mapping %s has auto-update enabled, a new mapping will be requested",
-                 map->toString().c_str());
-        // Run async to avoid double locks.
-        runOnUpnpContextThread([this, map] {
-            // Reserve a new mapping.
-            Mapping newMapping(*map);
-            newMapping.setExternalPort(0);
-            newMapping.setInternalPort(0);
-            reserveMapping(newMapping);
-            // Release the old one.
-            releaseMapping(*map);
-        });
-    }
 }
 
 #if HAVE_LIBNATPMP

src/upnp/upnp_context.h

@@ -221,8 +221,12 @@ private:
     void renewAllocations();
 #endif
 
+    // Process requests with pending status.
     void processPendingRequests(const std::shared_ptr<IGD>& igd);
 
+    // Process mapping with auto-update flag enabled.
+    void processMappingWithAutoUpdate();
+
     // Implementation of UpnpMappingObserver interface.
 
     // Callback used to report changes in IGD status.