diff --git a/ChangeLog b/ChangeLog index 9d1929425f..e684da4336 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 5067cb9a89..1dca9ec112 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 9f6e82707f..df02e112a4 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; }