diff options
author | Havard Graff <havard.graff@gmail.com> | 2017-03-30 10:17:08 +0300 |
---|---|---|
committer | Edward Hervey <bilboed@bilboed.com> | 2017-12-01 11:48:39 +0300 |
commit | b81223213f26ff96481a048222b3ff71dd87c4b5 (patch) | |
tree | 25af3d21f0c80bae17c0fb41c0c01c7db5a252ad | |
parent | 946b17ed10d0583a2c0e49b49573498c7069e02e (diff) |
ghostpad: fix race-condition while tearing down
An upstream query will take a ref on the internal proxypad, and can
hence end up owning the last reference to that pad, causing a crash.
-rw-r--r-- | gst/gstghostpad.c | 20 | ||||
-rw-r--r-- | tests/check/gst/gstghostpad.c | 46 |
2 files changed, 60 insertions, 6 deletions
diff --git a/gst/gstghostpad.c b/gst/gstghostpad.c index b03ae7dbd6..39755cfe90 100644 --- a/gst/gstghostpad.c +++ b/gst/gstghostpad.c @@ -506,13 +506,16 @@ gst_ghost_pad_dispose (GObject * object) GST_OBJECT_LOCK (pad); internal = GST_PROXY_PAD_INTERNAL (pad); + if (internal) { + gst_pad_set_activatemode_function (internal, NULL); - gst_pad_set_activatemode_function (internal, NULL); + GST_PROXY_PAD_INTERNAL (pad) = NULL; + GST_PROXY_PAD_INTERNAL (internal) = NULL; - /* disposes of the internal pad, since the ghostpad is the only possible object - * that has a refcount on the internal pad. */ - gst_object_unparent (GST_OBJECT_CAST (internal)); - GST_PROXY_PAD_INTERNAL (pad) = NULL; + /* disposes of the internal pad, since the ghostpad is the only possible object + * that has a refcount on the internal pad. */ + gst_object_unparent (GST_OBJECT_CAST (internal)); + } GST_OBJECT_UNLOCK (pad); @@ -832,7 +835,12 @@ gst_ghost_pad_set_target (GstGhostPad * gpad, GstPad * newtarget) g_return_val_if_fail (GST_IS_GHOST_PAD (gpad), FALSE); g_return_val_if_fail (GST_PAD_CAST (gpad) != newtarget, FALSE); - g_return_val_if_fail (newtarget != GST_PROXY_PAD_INTERNAL (gpad), FALSE); + + if (newtarget == GST_PROXY_PAD_INTERNAL (gpad)) { + GST_WARNING_OBJECT (gpad, "Target has already been set to %s:%s", + GST_DEBUG_PAD_NAME (newtarget)); + return FALSE; + } GST_OBJECT_LOCK (gpad); internal = GST_PROXY_PAD_INTERNAL (gpad); diff --git a/tests/check/gst/gstghostpad.c b/tests/check/gst/gstghostpad.c index 1a56ed382d..94ec14fbe5 100644 --- a/tests/check/gst/gstghostpad.c +++ b/tests/check/gst/gstghostpad.c @@ -1321,6 +1321,51 @@ GST_START_TEST (test_activate_sink_switch_mode) GST_END_TEST; +static gboolean thread_running; +static gpointer +send_query_to_pad_func (GstPad * pad) +{ + GstQuery *query = gst_query_new_latency (); + + while (thread_running) { + gst_pad_peer_query (pad, query); + g_thread_yield (); + } + + gst_query_unref (query); + return NULL; +} + +GST_START_TEST (test_stress_upstream_queries_while_tearing_down) +{ + GThread *query_thread; + gint i; + GstPad *pad = gst_pad_new ("sink", GST_PAD_SINK); + gst_pad_set_active (pad, TRUE); + + thread_running = TRUE; + query_thread = g_thread_new ("queries", + (GThreadFunc) send_query_to_pad_func, pad); + + for (i = 0; i < 1000; i++) { + GstPad *ghostpad = gst_ghost_pad_new ("ghost-sink", pad); + gst_pad_set_active (ghostpad, TRUE); + + g_thread_yield (); + + gst_ghost_pad_set_target (GST_GHOST_PAD_CAST (ghostpad), NULL); + gst_pad_set_active (pad, FALSE); + gst_object_unref (ghostpad); + } + + thread_running = FALSE; + g_thread_join (query_thread); + + gst_object_unref (pad); +} + +GST_END_TEST; + static Suite * gst_ghost_pad_suite (void) { @@ -1351,6 +1396,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_stress_upstream_queries_while_tearing_down); return s; } |