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/3651>
This commit is contained in:
Alicia Boya García 2019-05-04 03:54:44 +02:00 committed by Tim-Philipp Müller
parent e2f30cd947
commit ebe44034c1
2 changed files with 56 additions and 4 deletions

View file

@ -749,6 +749,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);
@ -773,10 +774,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);
@ -798,6 +797,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);