gstelement: fix deadlock in gst_element_add_pad() when >=PAUSED

gst_element_add_pad() is supposed to activate the pad if the element
state is >= PAUSED and the pad is not already active.

Unfortunately, before this patch, the activation was performed while the
element lock was still taken, which ended causing a deadlock in
gst_pad_start_task() as it attempted to post `stream-status` message in
the element, which also requires the element lock.

Elements could work around this bug by activating the pad manually
before adding it to the element.

This patch fixes the problem by performing pad activation only after the
element lock has been released.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/3635>
This commit is contained in:
Alicia Boya García 2019-05-04 03:54:44 +02:00 committed by GStreamer Marge Bot
parent 6540c4e89c
commit 15caeb4ac9
2 changed files with 56 additions and 4 deletions

View file

@ -747,6 +747,7 @@ gst_element_add_pad (GstElement * element, GstPad * pad)
{
gchar *pad_name;
gboolean active;
gboolean should_activate;
g_return_val_if_fail (GST_IS_ELEMENT (element), FALSE);
g_return_val_if_fail (GST_IS_PAD (pad), FALSE);
@ -771,10 +772,8 @@ gst_element_add_pad (GstElement * element, GstPad * pad)
goto had_parent;
/* check for active pads */
if (!active && (GST_STATE (element) > GST_STATE_READY ||
GST_STATE_NEXT (element) == GST_STATE_PAUSED)) {
gst_pad_set_active (pad, TRUE);
}
should_activate = !active && (GST_STATE (element) > GST_STATE_READY ||
GST_STATE_NEXT (element) == GST_STATE_PAUSED);
g_free (pad_name);
@ -796,6 +795,9 @@ gst_element_add_pad (GstElement * element, GstPad * pad)
element->pads_cookie++;
GST_OBJECT_UNLOCK (element);
if (should_activate)
gst_pad_set_active (pad, TRUE);
/* emit the PAD_ADDED signal */
g_signal_emit (element, gst_element_signals[PAD_ADDED], 0, pad);
GST_TRACER_ELEMENT_ADD_PAD (element, pad);

View file

@ -83,6 +83,55 @@ GST_START_TEST (test_add_pad_unref_element)
GST_END_TEST;
static void
test_add_pad_while_paused_dummy_task (void *user_data)
{
GstPad *pad = (GstPad *) user_data;
gst_pad_pause_task (pad);
}
static gboolean
test_add_pad_while_paused_pad_activatemode (GstPad * pad, GstObject * parent,
GstPadMode mode, gboolean active)
{
*(gboolean *) pad->activatemodedata = active;
fail_unless (mode == GST_PAD_MODE_PUSH);
if (active)
gst_pad_start_task (pad, test_add_pad_while_paused_dummy_task, pad, NULL);
else
gst_pad_stop_task (pad);
return TRUE;
}
GST_START_TEST (test_add_pad_while_paused)
{
GstElement *e;
GstPad *p;
gboolean active = FALSE;
e = gst_element_factory_make ("fakesrc", "source");
gst_element_set_state (e, GST_STATE_PAUSED);
{
GstPad *old_pad = gst_element_get_static_pad (e, "src");
gst_element_remove_pad (e, old_pad);
gst_object_unref (old_pad);
}
p = gst_pad_new ("dynamic", GST_PAD_SRC);
gst_pad_set_activatemode_function_full (p,
test_add_pad_while_paused_pad_activatemode, (void *) &active, NULL);
fail_if (active);
gst_element_add_pad (e, p);
fail_if (!active);
gst_element_set_state (e, GST_STATE_NULL);
fail_if (active);
gst_object_unref (e);
}
GST_END_TEST;
GST_START_TEST (test_error_no_bus)
{
GstElement *e;
@ -921,6 +970,7 @@ gst_element_suite (void)
suite_add_tcase (s, tc_chain);
tcase_add_test (tc_chain, test_add_remove_pad);
tcase_add_test (tc_chain, test_add_pad_unref_element);
tcase_add_test (tc_chain, test_add_pad_while_paused);
tcase_add_test (tc_chain, test_error_no_bus);
tcase_add_test (tc_chain, test_link);
tcase_add_test (tc_chain, test_link_no_pads);