upnp: Port mappings are not deleted upon IGD discovery anymore.

Part of the discovery event handling for libupnp was to delete
all port mappings associated with the local ip address of the
application. I can only assume this was done to close pre-
existing ports that were not properly closed when the application
terminated. The problem with this logic is that since
advertisements and discovery events are treated with the same
switch case (fall through), the application was trying to close
all the ports on the internet gateway everytime it got an
advertisement. However, the application would then try to reopen
the ports every time after closing them. And this would happen
every five to ten seconds (i.e. whenever the application would
catch and advertisement or discovery event from the internet
gateway.)

To fix this quickly, I modified the way the event handling treats
discovery and advertisement events. Instead of having a list based
on the URL of the internet gateway, I use a list of it's unique
service ID. That way, as soon as the event occurs, we check if
we don't already have this service ID in our list. If we don't,
then we proceed as usual. If we do, we exit the event handling
since it's already been processed.

The advertisement bye bye event has also been implemented. When
this event occurs, the corresponding internet gateway devices are
deleted from the lists.

Gitlab: #96

Change-Id: Idd8023eba319b431b3a9328ebe648e75d61b1dc8

src/upnp/upnp_context.cpp

@@ -958,12 +958,17 @@ UPnPContext::handleUPnPEvents(Upnp_EventType event_type, const void* event)
              JAMI_DBG("UPnP: CP received a discovery search result"); */
 
         /* check if we are already in the process of checking this device */
-        std::unique_lock<std::mutex> lock(cpDeviceMutex_);
-        auto it = cpDevices_.find(std::string(UpnpDiscovery_get_Location_cstr(d_event)));
+        std::lock_guard<std::mutex> lock(cpDeviceMutex_);
 
+        /*
+         * Check if this device ID is already in the list. If we reach the past-the-end
+         * iterator of the list, it means we haven't discovered it. So we add it.
+         */
+        std::string deviceId(UpnpDiscovery_get_DeviceID_cstr(d_event));
+        auto it = cpDevices_.find(deviceId);
         if (it == cpDevices_.end()) {
-            cpDevices_.emplace(std::string(UpnpDiscovery_get_Location_cstr(d_event)));
-            lock.unlock();
+            JAMI_DBG("PUPnP: New device ID found -> %s.", deviceId.c_str());
+            cpDevices_.emplace(deviceId);
 
             if (UpnpDiscovery_get_ErrCode(d_event) != UPNP_E_SUCCESS)
                 JAMI_WARN("UPnP: Error in discovery event received by the CP: %s",
@@ -991,18 +996,7 @@ UPnPContext::handleUPnPEvents(Upnp_EventType event_type, const void* event)
             } else {
                 parseDevice(desc_doc.get(), d_event);
             }
-
-            /* finished parsing device; remove it from know devices list,
-             * since next time it could be a new device with same URL
-             * eg: if we switch routers or if a new device with the same IP appears
-             */
-            lock.lock();
-            cpDevices_.erase(UpnpDiscovery_get_Location_cstr(d_event));
-            lock.unlock();
-        } else {
-            lock.unlock();
-            /* JAMI_DBG("UPnP: Control Point is already checking this device"); */
-        }
+        } 
     }
     break;
 
@@ -1018,6 +1012,18 @@ UPnPContext::handleUPnPEvents(Upnp_EventType event_type, const void* event)
                       UpnpGetErrorMessage(UpnpDiscovery_get_ErrCode(d_event)));
 
         /* TODO: check if its a device we care about and remove it from the relevant lists */
+        std::lock_guard<std::mutex> lock(cpDeviceMutex_);
+        std::string deviceId(UpnpDiscovery_get_DeviceID_cstr(d_event));
+        cpDevices_.erase(deviceId);
+
+        std::lock_guard<std::mutex> igd_list_lock(validIGDMutex_);
+        for (auto const& item : validIGDs_) {
+            if (((UPnPIGD*)item.second.get())->getUDN() == deviceId) {
+                validIGDs_.erase(item.first);
+                break;
+            }
+        }
+
     }
     break;