From 96d28a501ffd501c94de32ddf416d26dc63f358c Mon Sep 17 00:00:00 2001
From: Wim Taymans <wim.taymans@gmail.com>
Date: Fri, 21 Dec 2007 13:54:07 +0000
Subject: [PATCH] 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.
---
 ChangeLog                | 11 +++++++
 gst/gstpad.c             | 16 ++++++++--
 tests/check/gst/gstpad.c | 69 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 93 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 9d1929425f..e684da4336 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2007-12-21  Wim Taymans  <wim.taymans@collabora.co.uk>
+
+	* 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  <tim at centricular dot net>
 
 	Patch by: Peter Kjellerstedt <pkj axis com>
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;
 }