From af205f63b7172fcab61e846f354af23888bcf116 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Alicia=20Boya=20Garc=C3=ADa?= Date: Thu, 21 Feb 2019 22:01:24 +0100 Subject: [PATCH] gst-validate-scenario: Fix (another) race condition in EOS handling Since gst_validate_action_set_done() is asynchronous, the bus EOS handler may already be running before the action is actually finished. This patch ensures that is not a problem. --- validate/gst/validate/gst-validate-scenario.c | 29 ++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/validate/gst/validate/gst-validate-scenario.c b/validate/gst/validate/gst-validate-scenario.c index b6d235f..ec8a144 100644 --- a/validate/gst/validate/gst-validate-scenario.c +++ b/validate/gst/validate/gst-validate-scenario.c @@ -108,6 +108,7 @@ static GstValidateActionType *_find_action_type (const gchar * type_name); static GstValidateExecuteActionReturn _fill_action (GstValidateScenario * scenario, GstValidateAction * action, GstStructure * structure, gboolean add_to_lists); +static gboolean _action_set_done (GstValidateAction * action); /* GstValidateScenario is not really thread safe and * everything should be done from the thread GstValidate @@ -246,6 +247,7 @@ struct _GstValidateActionPrivate GWeakRef scenario; gboolean needs_playback_parsing; + gboolean pending_set_done; }; static JsonNode * @@ -3114,6 +3116,27 @@ message_cb (GstBus * bus, GstMessage * message, GstValidateScenario * scenario) GstStructure *s; GST_VALIDATE_SCENARIO_EOS_HANDLING_LOCK (scenario); + { + /* gst_validate_action_set_done() does not finish the action + * immediately. Instead, it posts a task to the main thread to do most + * of the work in _action_set_done(). + * + * While the EOS handling lock guarantees that if an action had to call + * gst_validate_action_set_done() it has done so, it does not guarantee + * that _action_set_done() has been called. + * + * Is it possible that this handler is run before _action_set_done(), so + * we check at this point for actions that have a pending_set_done and + * call it before continuing. */ + GList *actions = g_list_copy (priv->actions); + GList *i; + for (i = actions; i; i = i->next) { + GstValidateAction *action = (GstValidateAction *) i->data; + if (action->priv->pending_set_done) + _action_set_done (action); + } + g_list_free (actions); + } if (!is_error) { priv->got_eos = TRUE; @@ -4237,7 +4260,7 @@ _action_set_done (GstValidateAction * action) GstClockTime execution_duration; GstValidateScenario *scenario = gst_validate_action_get_scenario (action); - if (scenario == NULL) + if (scenario == NULL || !action->priv->pending_set_done) return G_SOURCE_REMOVE; execution_duration = gst_util_get_timestamp () - action->priv->execution_time; @@ -4268,6 +4291,7 @@ _action_set_done (GstValidateAction * action) } gst_object_unref (scenario); + action->priv->pending_set_done = FALSE; return G_SOURCE_REMOVE; } @@ -4298,6 +4322,9 @@ gst_validate_action_set_done (GstValidateAction * action) gst_validate_action_unref (action); } + g_assert (!action->priv->pending_set_done); + action->priv->pending_set_done = TRUE; + g_main_context_invoke_full (NULL, G_PRIORITY_DEFAULT_IDLE, (GSourceFunc) _action_set_done, gst_mini_object_ref (GST_MINI_OBJECT (action)), -- 2.7.4