pad: gst_pad_activate_mode() always succeed if same mode

Checking that the pad is in the correct mode before the parent is
checked makes the call always succeed if the mode is ok.

This fixes a race with ghostpad where gst_pad_activate_mode() could
trigger a g_critical() if the ghostpad is unparented while the
proxypad is deactivating, for instance if the ghostpad is released.
More specifically, gst_ghost_pad_internal_activate_push_default()'s
call to gst_pad_activate_mode() would fail if ghostpad doesn't have a
parent. With this patch it will return true of mode is already
correct.
This commit is contained in:
Stian Selnes 2017-08-18 14:30:32 +02:00 committed by Tim-Philipp Müller
parent df27ec3e67
commit 512cec3dea
2 changed files with 49 additions and 0 deletions

View file

@ -1303,11 +1303,19 @@ gst_pad_activate_mode (GstPad * pad, GstPadMode mode, gboolean active)
{
GstObject *parent;
gboolean res;
GstPadMode old, new;
g_return_val_if_fail (GST_IS_PAD (pad), FALSE);
GST_OBJECT_LOCK (pad);
old = GST_PAD_MODE (pad);
new = active ? mode : GST_PAD_MODE_NONE;
if (old == new)
goto was_ok;
ACQUIRE_PARENT (pad, parent, no_parent);
GST_OBJECT_UNLOCK (pad);
res = activate_mode_internal (pad, parent, mode, active);
@ -1316,6 +1324,13 @@ gst_pad_activate_mode (GstPad * pad, GstPadMode mode, gboolean active)
return res;
was_ok:
{
GST_OBJECT_UNLOCK (pad);
GST_CAT_DEBUG_OBJECT (GST_CAT_PADS, pad, "already %s in %s mode",
active ? "activated" : "deactivated", gst_pad_mode_get_name (mode));
return TRUE;
}
no_parent:
{
GST_WARNING_OBJECT (pad, "no parent");

View file

@ -1322,6 +1322,7 @@ GST_START_TEST (test_activate_sink_switch_mode)
GST_END_TEST;
static gboolean thread_running;
static gpointer
send_query_to_pad_func (GstPad * pad)
{
@ -1366,6 +1367,38 @@ GST_START_TEST (test_stress_upstream_queries_while_tearing_down)
GST_END_TEST;
GST_START_TEST (test_deactivate_already_deactive_with_no_parent)
{
/* This simulates the behavior where a ghostpad is released while
* deactivating (for instance because of a state change).
* gst_pad_activate_mode() may be be called from
* gst_ghost_pad_internal_activate_push_default() on a pad that is already
* deactivate and unparented. The call chain is really like somethink like
* this:
* gst_pad_activate_mode(ghostpad)
* -> ...
* -> gst_pad_activate_mode(proxypad)
* -> ...
* -> gst_pad_activate_mode(ghostpad)
*/
GstElement *bin = gst_bin_new ("testbin");
GstPad *pad = gst_ghost_pad_new_no_target ("src", GST_PAD_SRC);
gst_object_ref (pad);
/* We need to add/remove pad because that will update the pad's flags */
fail_unless (gst_element_add_pad (bin, pad));
fail_unless (gst_element_remove_pad (bin, pad));
/* Setting a pad that's already deactive to deactive should not fail. */
fail_if (gst_pad_is_active (pad));
fail_unless (gst_pad_activate_mode (pad, GST_PAD_MODE_PUSH, FALSE));
gst_object_unref (bin);
gst_object_unref (pad);
}
GST_END_TEST;
static Suite *
gst_ghost_pad_suite (void)
{
@ -1396,6 +1429,7 @@ gst_ghost_pad_suite (void)
tcase_add_test (tc_chain, test_activate_sink_and_src);
tcase_add_test (tc_chain, test_activate_src_pull_mode);
tcase_add_test (tc_chain, test_activate_sink_switch_mode);
tcase_add_test (tc_chain, test_deactivate_already_deactive_with_no_parent);
tcase_add_test (tc_chain, test_stress_upstream_queries_while_tearing_down);
return s;