[grilo-plugins] upnp: Fix a race condition when creating a new source

[Juan A. Suarez Romero <jasuarez src gnome org>]

commit 39437dfd621ab064c4f780f576a39d24f79600ae
Author: Juan A. Suarez Romero <jasuarez igalia com>
Date:   Tue Dec 4 19:16:54 2012 +0000

    upnp: Fix a race condition when creating a new source
    
    When creating a new source, before registering it, it is checked if the device
    supports search capability, so the source is registered with or without search
    operation, accordingly.
    
    Checking the capability is an asynchronous operation, and can happen that while
    that operation is running, the device disappears. This means that when the
    search capability operation finishes and call the proper callback, it must be
    checked if the device was removed or not. If it was, then drop the source, else
    register it.

 src/upnp/grl-upnp.c |   81 +++++++++++++++++++++++++++------------------------
 1 files changed, 43 insertions(+), 38 deletions(-)
---
diff --git a/src/upnp/grl-upnp.c b/src/upnp/grl-upnp.c
index 042f532..b8d7554 100644
--- a/src/upnp/grl-upnp.c
+++ b/src/upnp/grl-upnp.c
@@ -85,10 +85,7 @@ struct OperationSpec {
 };
 
 struct SourceInfo {
-  gchar *source_id;
-  gchar *source_name;
-  GUPnPDeviceProxy* device;
-  GUPnPServiceProxy* service;
+  GrlUpnpSource *source;
   GrlPlugin *plugin;
 };
 
@@ -141,6 +138,7 @@ static void device_unavailable_cb (GUPnPControlPoint *cp,
 static GHashTable *key_mapping = NULL;
 static GHashTable *filter_key_mapping = NULL;
 static GUPnPContextManager *context_manager = NULL;
+static GList *pending_sources = NULL;
 
 /* =================== UPnP Plugin  =============== */
 
@@ -265,10 +263,7 @@ build_source_id (const gchar *udn)
 static void
 free_source_info (struct SourceInfo *info)
 {
-  g_free (info->source_id);
-  g_free (info->source_name);
-  g_object_unref (info->device);
-  g_object_unref (info->service);
+  g_object_unref (info->source);
   g_object_unref (info->plugin);
   g_slice_free (struct SourceInfo, info);
 }
@@ -311,10 +306,7 @@ gupnp_search_caps_cb (GUPnPServiceProxy *service,
 {
   GError *error = NULL;
   gchar *caps = NULL;
-  gchar *name;
-  GrlUpnpSource *source;
-  gchar *source_id;
-  GrlRegistry *registry;
+  GrlSource *source;
   struct SourceInfo *source_info;
   gboolean result;
 
@@ -331,34 +323,30 @@ gupnp_search_caps_cb (GUPnPServiceProxy *service,
   }
 
   source_info = (struct SourceInfo *) user_data;
-  name = source_info->source_name;
-  source_id = source_info->source_id;
 
-  registry = grl_registry_get_default ();
-  if (grl_registry_lookup_source (registry, source_id)) {
-    GRL_DEBUG ("A source with id '%s' is already registered. Skipping...",
-               source_id);
+  /* Check if source has been removed (UPnP device not available) */
+  if (!g_list_find (pending_sources, source_info->source)) {
     goto free_resources;
   }
+  pending_sources = g_list_remove (pending_sources, source_info->source);
 
-  source = grl_upnp_source_new (source_id, name);
-  source->priv->device = g_object_ref (source_info->device);
-  source->priv->service = g_object_ref (source_info->service);
-
-  GRL_DEBUG ("Search caps for source '%s': '%s'", name, caps);
+  source = GRL_SOURCE (source_info->source);
 
   if (caps && caps[0] != '\0') {
-    GRL_DEBUG ("Setting search enabled for source '%s'", name );
-    source->priv->search_enabled = TRUE;
+    GRL_DEBUG ("Setting search enabled for source '%s'",
+               grl_source_get_name (source));
+    source_info->source->priv->search_enabled = TRUE;
   } else {
-    GRL_DEBUG ("Setting search disabled for source '%s'", name );
+    GRL_DEBUG ("Setting search disabled for source '%s'",
+               grl_source_get_name (source));
   }
 
-  grl_registry_register_source (registry,
+  grl_registry_register_source (grl_registry_get_default (),
                                 source_info->plugin,
-                                GRL_SOURCE (source),
+                                source,
                                 NULL);
 
+
  free_resources:
   g_free (caps);
   free_source_info (source_info);
@@ -431,19 +419,23 @@ device_available_cb (GUPnPControlPoint *cp,
 
   /* We got a valid UPnP source */
   /* Now let's check if it supports search operations before registering */
+  GrlUpnpSource *source = grl_upnp_source_new (source_id, name);
+  source->priv->device = g_object_ref (device);
+  source->priv->service = g_object_ref (service);
+
   struct SourceInfo *source_info = g_slice_new0 (struct SourceInfo);
-  source_info->source_id = g_strdup (source_id);
-  source_info->source_name = name;
-  source_info->device = g_object_ref (device);
-  source_info->service = g_object_ref (service);
+  source_info->source = g_object_ref (source);
   source_info->plugin = g_object_ref ((GrlPlugin *) user_data);
 
+  pending_sources = g_list_prepend (pending_sources, source);
+
   if (!gupnp_service_proxy_begin_action (GUPNP_SERVICE_PROXY (service),
 					 "GetSearchCapabilities",
 					 gupnp_search_caps_cb,
 					 source_info,
 					 NULL)) {
-    GrlUpnpSource *source = grl_upnp_source_new (source_id, name);
+    pending_sources = g_list_remove (pending_sources, source);
+    free_source_info (source_info);
     GRL_WARNING ("Failed to start GetCapabilitiesSearch action");
     GRL_DEBUG ("Setting search disabled for source '%s'", name );
     registry = grl_registry_get_default ();
@@ -451,7 +443,6 @@ device_available_cb (GUPnPControlPoint *cp,
                                   source_info->plugin,
                                   GRL_SOURCE (source),
                                   NULL);
-    free_source_info (source_info);
   }
 
  free_resources:
@@ -459,6 +450,13 @@ device_available_cb (GUPnPControlPoint *cp,
   g_free (source_id);
 }
 
+static gint
+source_matches_id (GrlSource *source,
+                   const gchar *source_id)
+{
+  return strcmp (grl_source_get_id (source), source_id);
+}
+
 static void
 device_unavailable_cb (GUPnPControlPoint *cp,
 		       GUPnPDeviceProxy *device,
@@ -468,6 +466,7 @@ device_unavailable_cb (GUPnPControlPoint *cp,
   GrlSource *source;
   GrlRegistry *registry;
   gchar *source_id;
+  GList *source_node;
 
   GRL_DEBUG ("device_unavailable_cb");
 
@@ -476,11 +475,17 @@ device_unavailable_cb (GUPnPControlPoint *cp,
 
   registry = grl_registry_get_default ();
   source_id = build_source_id (udn);
+  /* Check first if source is registered */
   source = grl_registry_lookup_source (registry, source_id);
-  if (!source) {
-    GRL_DEBUG ("No source registered with id '%s', ignoring", source_id);
-  } else {
-    grl_registry_unregister_source (registry, source, NULL);
+  if (source) {
+    GRL_DEBUG ("Unregistered source %s", source_id);
+    g_free (source_id);
+    return;
+  }
+
+  source_node = g_list_find_custom (pending_sources, source_id, (GCompareFunc) source_matches_id);
+  if (source_node) {
+    pending_sources = g_list_delete_link (pending_sources, source_node);
   }
 
   g_free (source_id);