[grilo-plugins] lua-factory: Fix concurrent Lua calls to same source

[Bastien Nocera <hadess src gnome org>]

commit fa88f91d329e2b1b6928406e24d633b6f12e005e
Author: Bastien Nocera <hadess hadess net>
Date:   Tue Mar 18 09:21:38 2014 +0100

    lua-factory: Fix concurrent Lua calls to same source
    
    This ensures that each fetch callback is called with the correct
    OperationSpec as the current one, so that we avoid leaking memory
    or crashing when 2 items overwrite the only store for OperationData.
    
    Before each lua_pcall(), set the current operation data,
    and unset it after.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=726563

 src/lua-factory/grl-lua-common.h  |    6 ++-
 src/lua-factory/grl-lua-factory.c |   14 +++++-
 src/lua-factory/grl-lua-library.c |  109 ++++++++++++++++++++++++++++++++++---
 3 files changed, 120 insertions(+), 9 deletions(-)
---
diff --git a/src/lua-factory/grl-lua-common.h b/src/lua-factory/grl-lua-common.h
index 47345d2..8ed1c57 100644
--- a/src/lua-factory/grl-lua-common.h
+++ b/src/lua-factory/grl-lua-common.h
@@ -67,6 +67,10 @@ typedef struct _OperationSpec {
 } OperationSpec;
 
 void grl_lua_library_save_operation_data (lua_State *L, OperationSpec *os);
-OperationSpec *grl_lua_library_load_operation_data (lua_State *L);
+void grl_lua_library_remove_operation_data (lua_State *L, guint operation_id);
+OperationSpec *grl_lua_library_load_operation_data (lua_State *L, guint operation_id);
+
+void grl_lua_library_set_current_operation (lua_State *L, guint operation_id);
+OperationSpec * grl_lua_library_get_current_operation (lua_State *L);
 
 #endif /* _GRL_LUA_LIBRARY_COMMON_H_ */
diff --git a/src/lua-factory/grl-lua-factory.c b/src/lua-factory/grl-lua-factory.c
index aa51fd9..6088a1b 100644
--- a/src/lua-factory/grl-lua-factory.c
+++ b/src/lua-factory/grl-lua-factory.c
@@ -988,6 +988,7 @@ grl_lua_factory_source_search (GrlSource *source,
   os->op_type = LUA_SEARCH;
 
   grl_lua_library_save_operation_data (L, os);
+  grl_lua_library_set_current_operation (L, os->operation_id);
   lua_getglobal (L, LUA_SOURCE_OPERATION[LUA_SEARCH]);
 
   text = (ss->text == NULL) ? "" : ss->text;
@@ -997,6 +998,8 @@ grl_lua_factory_source_search (GrlSource *source,
                  lua_tolstring (L, -1, NULL));
     lua_pop (L, 1);
   }
+
+  grl_lua_library_set_current_operation (L, 0);
 }
 
 static void
@@ -1022,6 +1025,7 @@ grl_lua_factory_source_browse (GrlSource *source,
   os->op_type = LUA_BROWSE;
 
   grl_lua_library_save_operation_data (L, os);
+  grl_lua_library_set_current_operation (L, os->operation_id);
   lua_getglobal (L, LUA_SOURCE_OPERATION[LUA_BROWSE]);
 
   media_id = grl_media_get_id (os->media);
@@ -1031,6 +1035,8 @@ grl_lua_factory_source_browse (GrlSource *source,
                  lua_tolstring (L, -1, NULL));
     lua_pop (L, 1);
   }
+
+  grl_lua_library_set_current_operation (L, 0);
 }
 
 static void
@@ -1055,6 +1061,7 @@ grl_lua_factory_source_query (GrlSource *source,
   os->op_type = LUA_QUERY;
 
   grl_lua_library_save_operation_data (L, os);
+  grl_lua_library_set_current_operation (L, os->operation_id);
   lua_getglobal (L, LUA_SOURCE_OPERATION[LUA_QUERY]);
 
   query = (qs->query == NULL) ? "" : qs->query;
@@ -1064,6 +1071,8 @@ grl_lua_factory_source_query (GrlSource *source,
                  lua_tolstring (L, -1, NULL));
     lua_pop (L, 1);
   }
+
+  grl_lua_library_set_current_operation (L, 0);
 }
 
 static void
@@ -1076,7 +1085,7 @@ grl_lua_factory_source_resolve (GrlSource *source,
 
   GRL_DEBUG ("grl_lua_factory_source_resolve");
 
-  g_return_if_fail (grl_lua_library_load_operation_data (L) == NULL);
+  g_return_if_fail (grl_lua_library_load_operation_data (L, rs->operation_id) == NULL);
 
   os = g_slice_new0 (OperationSpec);
   os->source = rs->source;
@@ -1090,6 +1099,7 @@ grl_lua_factory_source_resolve (GrlSource *source,
   os->op_type = LUA_RESOLVE;
 
   grl_lua_library_save_operation_data (L, os);
+  grl_lua_library_set_current_operation (L, os->operation_id);
   lua_getglobal (L, LUA_SOURCE_OPERATION[LUA_RESOLVE]);
 
   if (lua_pcall (L, 0, 0, 0)) {
@@ -1097,6 +1107,8 @@ grl_lua_factory_source_resolve (GrlSource *source,
                  lua_tolstring (L, -1, NULL));
     lua_pop (L, 1);
   }
+
+  grl_lua_library_set_current_operation (L, 0);
 }
 
 static gboolean
diff --git a/src/lua-factory/grl-lua-library.c b/src/lua-factory/grl-lua-library.c
index 936c512..a47eb85 100644
--- a/src/lua-factory/grl-lua-library.c
+++ b/src/lua-factory/grl-lua-library.c
@@ -34,6 +34,7 @@ GRL_LOG_DOMAIN_STATIC (lua_library_log_domain);
 
 typedef struct _FetchOperation {
   lua_State *L;
+  guint operation_id;
   gchar *lua_cb;
   guint index;
   gchar *url;
@@ -327,6 +328,8 @@ grl_util_fetch_done (GObject *source_object,
     }
   }
 
+  grl_lua_library_set_current_operation (L, fo->operation_id);
+
   lua_getglobal (L, fo->lua_cb);
 
   if (!fo->is_table) {
@@ -345,6 +348,8 @@ grl_util_fetch_done (GObject *source_object,
                  fo->lua_cb, lua_tolstring (L, -1, NULL));
   }
 
+  grl_lua_library_set_current_operation (L, 0);
+
   for (i = 0; i < fo->num_urls; i++)
     g_free (fo->results[i]);
   g_free (fo->url);
@@ -370,7 +375,8 @@ grl_l_operation_get_options (lua_State *L)
 
   luaL_argcheck (L, lua_isstring (L, 1), 1, "expecting option (string)");
 
-  os = grl_lua_library_load_operation_data (L);
+  os = grl_lua_library_get_current_operation (L);
+  g_return_val_if_fail (os != NULL, 0);
   option = lua_tostring (L, 1);
 
   if (g_strcmp0 (option, "count") == 0) {
@@ -486,7 +492,8 @@ grl_l_operation_get_keys (lua_State *L)
   const gchar *key_name = NULL;
   gint i = 0;
 
-  os = grl_lua_library_load_operation_data (L);
+  os = grl_lua_library_get_current_operation (L);
+  g_return_val_if_fail (os != NULL, 0);
 
   registry = grl_registry_get_default ();
   lua_newtable (L);
@@ -518,7 +525,8 @@ grl_l_media_get_keys (lua_State *L)
   GrlKeyID key_id;
   gchar *key_name = NULL;
 
-  os = grl_lua_library_load_operation_data (L);
+  os = grl_lua_library_get_current_operation (L);
+  g_return_val_if_fail (os != NULL, 0);
 
   registry = grl_registry_get_default ();
   lua_newtable (L);
@@ -590,12 +598,16 @@ grl_l_fetch (lua_State *L)
   GrlNetWc *wc = NULL;
   FetchOperation *fo = NULL;
   gboolean is_table = FALSE;
+  OperationSpec *os;
 
   luaL_argcheck (L, (lua_isstring (L, 1) || lua_istable (L, 1)), 1,
                  "expecting url as string or an array of urls");
   luaL_argcheck (L, lua_isstring (L, 2), 2,
                  "expecting callback function as string");
 
+  os = grl_lua_library_get_current_operation (L);
+  g_return_val_if_fail (os != NULL, 0);
+
   num_urls = (lua_isstring (L, 1)) ? 1 : luaL_len (L, 1);
   urls = g_new0 (gchar *, num_urls);
 
@@ -660,6 +672,7 @@ grl_l_fetch (lua_State *L)
   for (i = 0; i < num_urls; i++) {
     fo = g_new0 (FetchOperation, 1);
     fo->L = L;
+    fo->operation_id = os->operation_id;
     fo->lua_cb = g_strdup (lua_callback);
     fo->index = i;
     fo->url = g_strdup (urls[i]);
@@ -692,7 +705,7 @@ grl_l_callback (lua_State *L)
   GRL_DEBUG ("grl.callback()");
 
   nparam = lua_gettop (L);
-  os = grl_lua_library_load_operation_data (L);
+  os = grl_lua_library_get_current_operation (L);
   g_return_val_if_fail (os != NULL, 0);
 
   media = (os->op_type == LUA_RESOLVE) ? os->media : NULL;
@@ -715,8 +728,8 @@ grl_l_callback (lua_State *L)
   if (count == 0) {
     g_list_free (os->keys);
     g_object_unref (os->options);
+    grl_lua_library_remove_operation_data (L, os->operation_id);
     g_slice_free (OperationSpec, os);
-    grl_lua_library_save_operation_data (L, NULL);
   }
 
   return 0;
@@ -883,21 +896,102 @@ luaopen_grilo (lua_State *L)
 void
 grl_lua_library_save_operation_data (lua_State *L, OperationSpec *os)
 {
+  char *op_id;
+
+  g_return_if_fail (os != NULL);
+
+  op_id = g_strdup_printf (GRILO_LUA_OPERATION_INDEX "-%i", os->operation_id);
   lua_getglobal (L, LUA_ENV_TABLE);
-  lua_pushstring (L, GRILO_LUA_OPERATION_INDEX);
+  lua_pushstring (L, op_id);
   lua_pushlightuserdata (L, os);
   lua_settable (L, -3);
   lua_pop (L, 1);
+  g_free (op_id);
+}
+
+/**
+ * grl_lua_library_remove_operation_data
+ *
+ * @L: LuaState where the data will be removed.
+ * @operation_id: The operation ID to remove.
+ * @return: Nothing.
+ *
+ * Remove the OperationSpec with this ID from the the global environment of
+ * lua_State.
+ **/
+void
+grl_lua_library_remove_operation_data (lua_State *L, guint operation_id)
+{
+  char *op_id;
+
+  op_id = g_strdup_printf (GRILO_LUA_OPERATION_INDEX "-%i", operation_id);
+  lua_getglobal (L, LUA_ENV_TABLE);
+  lua_pushstring (L, op_id);
+  lua_pushlightuserdata (L, NULL);
+  lua_settable (L, -3);
+  lua_pop (L, 1);
+  g_free (op_id);
 }
 
 /**
  * grl_lua_library_load_operation_data
  *
  * @L : LuaState where the data is stored.
+ * @operation_id: The operation ID to load Operation Data for.
+ * to load the Operation Data for the current call.
  * @return: The Operation Data.
  **/
 OperationSpec *
-grl_lua_library_load_operation_data (lua_State *L)
+grl_lua_library_load_operation_data (lua_State *L, guint operation_id)
+{
+  OperationSpec *os = NULL;
+  char *op_id;
+
+  g_return_val_if_fail (operation_id > 0, NULL);
+
+  op_id = g_strdup_printf (GRILO_LUA_OPERATION_INDEX "-%i", operation_id);
+  lua_getglobal (L, LUA_ENV_TABLE);
+  lua_pushstring (L, op_id);
+  lua_gettable (L, -2);
+  os = (lua_islightuserdata(L, -1)) ? lua_touserdata(L, -1) : NULL;
+  lua_pop(L, 1);
+  g_free (op_id);
+
+  return os;
+}
+
+/**
+ * grl_lua_library_set_current_operation
+ *
+ * @L: LuaState where the data is stored.
+ * @operation_id: The current operation ID.
+ * @return: Nothing:
+ **/
+void
+grl_lua_library_set_current_operation (lua_State *L, guint operation_id)
+{
+  OperationSpec *os;
+
+  if (operation_id > 0)
+    os = grl_lua_library_load_operation_data (L, operation_id);
+  else
+    os = NULL;
+
+  lua_getglobal (L, LUA_ENV_TABLE);
+  lua_pushstring (L, GRILO_LUA_OPERATION_INDEX);
+  lua_pushlightuserdata (L, os);
+  lua_settable (L, -3);
+  lua_pop (L, 1);
+}
+
+/**
+ * grl_lua_library_get_current_operation
+ *
+ * @L: LuaState where the data is stored.
+ * @return: The Operation Data for the current operation.
+ **/
+OperationSpec *
+grl_lua_library_get_current_operation (lua_State *L)
 {
   OperationSpec *os = NULL;
 
@@ -906,5 +1000,6 @@ grl_lua_library_load_operation_data (lua_State *L)
   lua_gettable (L, -2);
   os = (lua_islightuserdata(L, -1)) ? lua_touserdata(L, -1) : NULL;
   lua_pop(L, 1);
+
   return os;
 }