From: Wim Taymans Date: Fri, 21 Dec 2007 13:54:07 +0000 (+0000) Subject: gst/gstpad.c: Really unlink the peer pad instead of setting the peer pointer to NULL... X-Git-Tag: RELEASE-0_10_16~47 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=96d28a501ffd501c94de32ddf416d26dc63f358c;p=platform%2Fupstream%2Fgstreamer.git gst/gstpad.c: Really unlink the peer pad instead of setting the peer pointer to NULL when we dispose the pad. Original commit message from CVS: * gst/gstpad.c: (gst_pad_dispose): Really unlink the peer pad instead of setting the peer pointer to NULL when we dispose the pad. This correctly calls the unlink functions and makes sure that the peer does not have a handle to invalid memory. See #504671. * tests/check/gst/gstpad.c: (GST_START_TEST), (gst_pad_suite): Add testsuite for above case. --- diff --git a/ChangeLog b/ChangeLog index 9d19294..e684da4 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,14 @@ +2007-12-21 Wim Taymans + + * gst/gstpad.c: (gst_pad_dispose): + Really unlink the peer pad instead of setting the peer pointer to NULL + when we dispose the pad. + This correctly calls the unlink functions and makes sure that the peer + does not have a handle to invalid memory. See #504671. + + * tests/check/gst/gstpad.c: (GST_START_TEST), (gst_pad_suite): + Add testsuite for above case. + 2007-12-20 Tim-Philipp Müller Patch by: Peter Kjellerstedt diff --git a/gst/gstpad.c b/gst/gstpad.c index 5067cb9..1dca9ec 100644 --- a/gst/gstpad.c +++ b/gst/gstpad.c @@ -367,12 +367,22 @@ static void gst_pad_dispose (GObject * object) { GstPad *pad = GST_PAD (object); + GstPad *peer; GST_CAT_DEBUG_OBJECT (GST_CAT_REFCOUNTING, pad, "dispose"); - /* we don't hold a ref to the peer so we can just set the - * peer to NULL. */ - GST_PAD_PEER (pad) = NULL; + /* unlink the peer pad */ + if ((peer = gst_pad_get_peer (pad))) { + /* window for MT unsafeness, someone else could unlink here + * and then we call unlink with wrong pads. The unlink + * function would catch this and safely return failed. */ + if (GST_PAD_IS_SRC (pad)) + gst_pad_unlink (pad, peer); + else + gst_pad_unlink (peer, pad); + + gst_object_unref (peer); + } /* clear the caps */ gst_caps_replace (&GST_PAD_CAPS (pad), NULL); diff --git a/tests/check/gst/gstpad.c b/tests/check/gst/gstpad.c index 9f6e827..df02e11 100644 --- a/tests/check/gst/gstpad.c +++ b/tests/check/gst/gstpad.c @@ -510,6 +510,73 @@ GST_START_TEST (test_push_negotiation) GST_END_TEST; +/* see that an unref also unlinks the pads */ +GST_START_TEST (test_src_unref_unlink) +{ + GstPad *src, *sink; + GstCaps *caps; + GstPadLinkReturn plr; + + sink = gst_pad_new ("sink", GST_PAD_SINK); + fail_if (sink == NULL); + + src = gst_pad_new ("src", GST_PAD_SRC); + fail_if (src == NULL); + + caps = gst_caps_from_string ("foo/bar"); + + gst_pad_set_caps (src, caps); + gst_pad_set_caps (sink, caps); + + plr = gst_pad_link (src, sink); + fail_unless (GST_PAD_LINK_SUCCESSFUL (plr)); + + /* unref the srcpad */ + gst_object_unref (src); + + /* sink should be unlinked now */ + fail_if (gst_pad_is_linked (sink)); + + /* cleanup */ + gst_object_unref (sink); + gst_caps_unref (caps); +} + +GST_END_TEST; + +/* see that an unref also unlinks the pads */ +GST_START_TEST (test_sink_unref_unlink) +{ + GstPad *src, *sink; + GstCaps *caps; + GstPadLinkReturn plr; + + sink = gst_pad_new ("sink", GST_PAD_SINK); + fail_if (sink == NULL); + + src = gst_pad_new ("src", GST_PAD_SRC); + fail_if (src == NULL); + + caps = gst_caps_from_string ("foo/bar"); + + gst_pad_set_caps (src, caps); + gst_pad_set_caps (sink, caps); + + plr = gst_pad_link (src, sink); + fail_unless (GST_PAD_LINK_SUCCESSFUL (plr)); + + /* unref the sinkpad */ + gst_object_unref (sink); + + /* src should be unlinked now */ + fail_if (gst_pad_is_linked (src)); + + /* cleanup */ + gst_object_unref (src); + gst_caps_unref (caps); +} + +GST_END_TEST; Suite * gst_pad_suite (void) @@ -530,6 +597,8 @@ gst_pad_suite (void) tcase_add_test (tc_chain, test_push_linked); tcase_add_test (tc_chain, test_flowreturn); tcase_add_test (tc_chain, test_push_negotiation); + tcase_add_test (tc_chain, test_src_unref_unlink); + tcase_add_test (tc_chain, test_sink_unref_unlink); return s; }