From 03dc22951bacb6fdc3868c8f801e6a52c33a745f Mon Sep 17 00:00:00 2001
From: Mathieu Duponchelle <mathieu@centricular.com>
Date: Mon, 15 Jan 2018 18:13:37 +0100
Subject: rtpbin: fix leak of elements requested by signals

When the signal returns a floating reference, as its return type
is transfer full, we need to sink it ourselves before passing
it to gst_bin_add (which is transfer floating).

This allows us to unref it in bin_remove_element later on, and
thus to also release the reference we now own if the signal
returns a non-floating reference as well.

As we now still hold a reference to the element when removing it,
we also need to lock its state and setting it to NULL before
unreffing it

Also update the request_aux_sender test.

https://bugzilla.gnome.org/show_bug.cgi?id=792543
---
 gst/rtpmanager/gstrtpbin.c    | 14 +++++++++++---
 tests/check/elements/rtpbin.c | 16 ++++++++++++++--
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/gst/rtpmanager/gstrtpbin.c b/gst/rtpmanager/gstrtpbin.c
index 9b2c0396d..d848711a3 100644
--- a/gst/rtpmanager/gstrtpbin.c
+++ b/gst/rtpmanager/gstrtpbin.c
@@ -865,6 +865,10 @@ bin_manage_element (GstRtpBin * bin, GstElement * element)
     GST_DEBUG_OBJECT (bin, "requested element %p already in bin", element);
   } else {
     GST_DEBUG_OBJECT (bin, "adding requested element %p", element);
+
+    if (g_object_is_floating (element))
+      element = gst_object_ref_sink (element);
+
     if (!gst_bin_add (GST_BIN_CAST (bin), element))
       goto add_failed;
     if (!gst_element_sync_state_with_parent (element))
@@ -880,6 +884,7 @@ bin_manage_element (GstRtpBin * bin, GstElement * element)
 add_failed:
   {
     GST_WARNING_OBJECT (bin, "unable to add element");
+    gst_object_unref (element);
     return FALSE;
   }
 }
@@ -894,10 +899,13 @@ remove_bin_element (GstElement * element, GstRtpBin * bin)
   if (find) {
     priv->elements = g_list_delete_link (priv->elements, find);
 
-    if (!g_list_find (priv->elements, element))
+    if (!g_list_find (priv->elements, element)) {
+      gst_element_set_locked_state (element, TRUE);
       gst_bin_remove (GST_BIN_CAST (bin), element);
-    else
-      gst_object_unref (element);
+      gst_element_set_state (element, GST_STATE_NULL);
+    }
+
+    gst_object_unref (element);
   }
 }
 
diff --git a/tests/check/elements/rtpbin.c b/tests/check/elements/rtpbin.c
index 8a72e3655..981cd6487 100644
--- a/tests/check/elements/rtpbin.c
+++ b/tests/check/elements/rtpbin.c
@@ -576,7 +576,7 @@ aux_sender_cb (GstElement * rtpbin, guint sessid, gpointer user_data)
   GstElement *bin;
   GstPad *srcpad, *sinkpad;
 
-  bin = gst_bin_new (NULL);
+  bin = (GstElement *) user_data;
 
   GST_DEBUG ("making AUX sender");
   sinkpad = gst_ghost_pad_new_no_target ("sink_2", GST_PAD_SINK);
@@ -597,11 +597,14 @@ GST_START_TEST (test_aux_sender)
   GstElement *rtpbin;
   GstPad *rtp_sink1, *rtp_src, *rtcp_src;
   gulong id;
+  GstElement *aux_sender = gst_object_ref_sink (gst_bin_new ("aux-sender"));
+
+  gst_object_ref (aux_sender);
 
   rtpbin = gst_element_factory_make ("rtpbin", "rtpbin");
 
   id = g_signal_connect (rtpbin, "request-aux-sender",
-      (GCallback) aux_sender_cb, NULL);
+      (GCallback) aux_sender_cb, aux_sender);
 
   rtp_sink1 = gst_element_get_request_pad (rtpbin, "send_rtp_sink_2");
   fail_unless (rtp_sink1 != NULL);
@@ -631,6 +634,15 @@ GST_START_TEST (test_aux_sender)
   gst_element_release_request_pad (rtpbin, rtp_sink1);
   gst_object_unref (rtp_sink1);
 
+  /* We have sinked the initial reference before returning it
+   * in the request callback, the ref count should now be 1 because
+   * the return of the signal is transfer full, and rtpbin should
+   * have released that reference by now, but we had taken an
+   * extra reference to perform this check
+   */
+  ASSERT_OBJECT_REFCOUNT (aux_sender, "aux-sender", 1);
+
+  gst_object_unref (aux_sender);
   gst_object_unref (rtpbin);
 }
 
-- 
cgit v1.2.3