validate-scenario: Use thread-safe GWeakRef
authorOlivier Crête <olivier.crete@collabora.com>
Fri, 24 Jul 2015 20:47:57 +0000 (16:47 -0400)
committerOlivier Crête <olivier.crete@collabora.com>
Fri, 24 Jul 2015 23:59:20 +0000 (19:59 -0400)
Since _set_done() is meant to be thread safe,
it can not be used with g_object_add_weak_pointer(),
instead, one must use GWeakRef. But since it is in the API,
document that fact and add a couple assertions to make sure
it doesn't get broken in the future.

validate/gst/validate/gst-validate-scenario.c
validate/gst/validate/gst-validate-scenario.h

index 0aae6b3..cefca19 100644 (file)
@@ -189,6 +189,8 @@ struct _GstValidateActionPrivate
   gboolean printed;
   gboolean executing_last_subaction;
   gboolean optional;
+
+  GWeakRef scenario;
 };
 
 GST_DEFINE_MINI_OBJECT_TYPE (GstValidateAction, gst_validate_action);
@@ -231,6 +233,8 @@ _action_free (GstValidateAction * action)
     g_object_remove_weak_pointer (G_OBJECT (action->scenario),
         ((gpointer *) & action->scenario));
 
+  g_weak_ref_clear (&action->priv->scenario);
+
   g_slice_free (GstValidateActionPrivate, action->priv);
   g_slice_free (GstValidateAction, action);
 }
@@ -243,6 +247,8 @@ gst_validate_action_init (GstValidateAction * action)
       (GstMiniObjectFreeFunction) _action_free);
 
   action->priv = g_slice_new0 (GstValidateActionPrivate);
+
+  g_weak_ref_init (&action->priv->scenario, NULL);
 }
 
 static void
@@ -262,6 +268,7 @@ gst_validate_action_new (GstValidateScenario * scenario,
   action->type = action_type->name;
   action->repeat = -1;
 
+  g_weak_ref_set (&action->priv->scenario, scenario);
   action->scenario = scenario;
   if (scenario)
     g_object_add_weak_pointer (G_OBJECT (scenario),
@@ -2176,6 +2183,11 @@ static void
 _pipeline_freed_cb (GstValidateScenario * scenario,
     GObject * where_the_object_was)
 {
+  /* Because g_object_weak_ref() is used, this MUST be on the
+   * main thread. */
+  g_assert (g_main_context_acquire (g_main_context_default ()));
+  g_main_context_release (g_main_context_default ());
+
   scenario->pipeline = NULL;
 
   GST_DEBUG_OBJECT (scenario, "pipeline was freed");
@@ -2508,6 +2520,11 @@ gst_validate_scenario_finalize (GObject * object)
 {
   GstValidateScenarioPrivate *priv = GST_VALIDATE_SCENARIO (object)->priv;
 
+  /* Because g_object_add_weak_pointer() is used, this MUST be on the
+   * main thread. */
+  g_assert (g_main_context_acquire (g_main_context_default ()));
+  g_main_context_release (g_main_context_default ());
+
   g_list_free_full (priv->actions, (GDestroyNotify) gst_mini_object_unref);
   g_list_free_full (priv->interlaced_actions,
       (GDestroyNotify) gst_mini_object_unref);
@@ -2912,18 +2929,22 @@ _action_set_done (GstValidateAction * action)
 void
 gst_validate_action_set_done (GstValidateAction * action)
 {
-  GstValidateScenario *scenario = action->scenario;
 
   if (action->priv->state == GST_VALIDATE_EXECUTE_ACTION_INTERLACED) {
+    GstValidateScenario *scenario = g_weak_ref_get (&action->priv->scenario);
+    GList *item = NULL;
 
     if (scenario) {
       SCENARIO_LOCK (scenario);
+      item = g_list_find (scenario->priv->interlaced_actions, action);
       scenario->priv->interlaced_actions =
-          g_list_remove (scenario->priv->interlaced_actions, action);
+          g_list_delete_link (scenario->priv->interlaced_actions, item);
       SCENARIO_UNLOCK (scenario);
+      g_object_unref (scenario);
     }
 
-    gst_validate_action_unref (action);
+    if (item)
+      gst_validate_action_unref (action);
   }
 
   g_main_context_invoke_full (NULL, G_PRIORITY_DEFAULT_IDLE,
index c4a7f80..b04d7e6 100644 (file)
@@ -88,8 +88,11 @@ typedef struct _GstValidateActionPrivate          GstValidateActionPrivate;
  *        #gst_validate_register_action_type
  * @name: The name of the action, set from the user in the scenario
  * @structure: the #GstStructure defining the action
+ * @scenario: The scenario for this action
  *
  * The GstValidateAction defined to be executed as part of a scenario
+ *
+ * Only access it from the default main context.
  */
 struct _GstValidateAction
 {