From 62047a9f8daf1c3ca218864fd7f755f9c024dd3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim-Philipp=20M=C3=BCller?= Date: Wed, 19 Jun 2024 01:55:57 +0100 Subject: [PATCH] rtpdtmfsrc: fix leak when shutting down mid-event .. and update rtpdtmfdepay unit test to trigger the potential leak more reliably (without the fix). Fixes https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/3633 Part-of: --- .../gst-plugins-good/gst/dtmf/gstrtpdtmfsrc.c | 3 ++ .../tests/check/elements/dtmf.c | 41 ++++++++++--------- 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/subprojects/gst-plugins-good/gst/dtmf/gstrtpdtmfsrc.c b/subprojects/gst-plugins-good/gst/dtmf/gstrtpdtmfsrc.c index 0760b54669..2fbcfbfed0 100644 --- a/subprojects/gst-plugins-good/gst/dtmf/gstrtpdtmfsrc.c +++ b/subprojects/gst-plugins-good/gst/dtmf/gstrtpdtmfsrc.c @@ -1084,6 +1084,9 @@ gst_rtp_dtmf_src_change_state (GstElement * element, GstStateChange transition) } dtmfsrc->last_event_was_start = FALSE; + // Clear out any unfinished events + g_clear_pointer (&dtmfsrc->payload, g_free); + /* Indicate that we don't do PRE_ROLL */ break; diff --git a/subprojects/gst-plugins-good/tests/check/elements/dtmf.c b/subprojects/gst-plugins-good/tests/check/elements/dtmf.c index 863717efb4..54025bbbdb 100644 --- a/subprojects/gst-plugins-good/tests/check/elements/dtmf.c +++ b/subprojects/gst-plugins-good/tests/check/elements/dtmf.c @@ -562,31 +562,34 @@ GST_END_TEST; GST_START_TEST (test_rtpdtmfdepay_src_caps_fixated) { - GstElement *pipeline; - GstStructure *s; + // Loop a few times, additional test for rtpdtmfsrc leak on early shutdown + for (int i = 0; i < 50; ++i) { + GstElement *pipeline; + GstStructure *s; - pipeline = - gst_parse_launch ("rtpdtmfsrc ! rtpdtmfdepay ! audioconvert ! fakesink", - NULL); + pipeline = + gst_parse_launch ("rtpdtmfsrc ! rtpdtmfdepay ! audioconvert ! fakesink", + NULL); - fail_unless_equals_int (gst_element_set_state (pipeline, GST_STATE_PLAYING), - GST_STATE_CHANGE_ASYNC); + fail_unless_equals_int (gst_element_set_state (pipeline, GST_STATE_PLAYING), + GST_STATE_CHANGE_ASYNC); - // Send an event so there's some data flow and the pipeline prerolls - s = gst_structure_new ("dtmf-event", - "type", G_TYPE_INT, 1, "number", G_TYPE_INT, 3, - "method", G_TYPE_INT, 1, "volume", G_TYPE_INT, 8, - "start", G_TYPE_BOOLEAN, TRUE, NULL); + // Send an event so there's some data flow and the pipeline prerolls + s = gst_structure_new ("dtmf-event", + "type", G_TYPE_INT, 1, "number", G_TYPE_INT, 3, + "method", G_TYPE_INT, 1, "volume", G_TYPE_INT, 8, + "start", G_TYPE_BOOLEAN, TRUE, NULL); - fail_unless (gst_element_send_event (pipeline, - gst_event_new_custom (GST_EVENT_CUSTOM_UPSTREAM, s))); + fail_unless (gst_element_send_event (pipeline, + gst_event_new_custom (GST_EVENT_CUSTOM_UPSTREAM, s))); - // Wait for preroll - fail_unless_equals_int (gst_element_get_state (pipeline, NULL, NULL, -1), - GST_STATE_CHANGE_SUCCESS); + // Wait for preroll + fail_unless_equals_int (gst_element_get_state (pipeline, NULL, NULL, -1), + GST_STATE_CHANGE_SUCCESS); - gst_element_set_state (pipeline, GST_STATE_NULL); - gst_object_unref (pipeline); + gst_element_set_state (pipeline, GST_STATE_NULL); + gst_object_unref (pipeline); + } } GST_END_TEST;