After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 765005 - mpegtsmux: memory leak caused by pad_data->language
mpegtsmux: memory leak caused by pad_data->language
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
1.6.1
Other Linux
: Normal normal
: 1.8.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-04-13 17:28 UTC by Damian Ziobro
Modified: 2016-04-15 20:18 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Solving memory leak in mpegtsmux by freeing pad_data->language before g_strdup (960 bytes, patch)
2016-04-13 17:30 UTC, Damian Ziobro
none Details | Review
Solving memory leak in mpegtsmux by freeing pad_data->language before g_strdup - new (960 bytes, patch)
2016-04-13 17:58 UTC, Damian Ziobro
committed Details | Review

Description Damian Ziobro 2016-04-13 17:28:36 UTC
I noticed small memory leak caused by pad_data->language in mpegtsmux in the following line:

          GST_DEBUG_OBJECT (pad, "Setting language to '%s'", lang_code);
          pad_data->language = g_strdup (lang_code);

I added to WARNING messages to confirm that g_strdup is invoked more times that g_free(pad_data->language) in the following places:

1)
-         GST_DEBUG_OBJECT (pad, "Setting language to '%s'", lang_code);
+         GST_WARNING_OBJECT (pad, "Setting language to '%s'", lang_code);
          pad_data->language = g_strdup (lang_code);

2)
  if (pad_data->language) {
+   GST_WARNING(releasing pad_data->language);
    g_free (pad_data->language);
    pad_data->language = NULL;
  }


And got following output in logs:

0:00:10.753865376  9671      0x9cd96c0 WARN               mpegtsmux mpegtsmux.c:356:mpegtsmux_pad_reset: releasing pad_data->language
0:00:10.755888141  9671      0x9cd96c0 WARN               mpegtsmux mpegtsmux.c:869:mpegtsmux_sink_event <splitmuxsink_rabbitmq_mpegtsmux_4287:sink_66> Setting language to 'eng'
0:00:10.775769785  9671      0x9cd96c0 WARN               mpegtsmux mpegtsmux.c:869:mpegtsmux_sink_event:<splitmuxsink_rabbitmq_mpegtsmux_4287:sink_66> Setting language to 'eng'
0:00:11.211465160  9671      0x9b7e1b0 WARN               mpegtsmux mpegtsmux.c:356:mpegtsmux_pad_reset: releasing pad_data->language
0:00:11.213284822  9671      0x9b7e1b0 WARN               mpegtsmux mpegtsmux.c:869:mpegtsmux_sink_event:<splitmuxsink_rabbitmq_mpegtsmux_4352:sink_66> Setting language to 'eng'
0:00:11.220769453  9671      0x9b7fb80 WARN               mpegtsmux mpegtsmux.c:869:mpegtsmux_sink_event:<splitmuxsink_rabbitmq_mpegtsmux_4352:sink_66> Setting language to 'eng'
0:00:11.408445647  9671      0x9ce37b0 WARN               mpegtsmux mpegtsmux.c:356:mpegtsmux_pad_reset: releasing pad_data->language
0:00:11.411094371  9671      0x9ce37b0 WARN               mpegtsmux mpegtsmux.c:869:mpegtsmux_sink_event:<splitmuxsink_rabbitmq_mpegtsmux_4170:sink_66> Setting language to 'eng'
0:00:11.428846042  9671      0x9ce37b0 WARN               mpegtsmux mpegtsmux.c:869:mpegtsmux_sink_event:<splitmuxsink_rabbitmq_mpegtsmux_4170:sink_66> Setting language to 'eng'
0:00:11.622453320  9671      0x9b04150 WARN               mpegtsmux mpegtsmux.c:356:mpegtsmux_pad_reset: releasing pad_data->language
0:00:11.624472643  9671      0x9b04150 WARN               mpegtsmux mpegtsmux.c:869:mpegtsmux_sink_event:<splitmuxsink_rabbitmq_mpegtsmux_4736:sink_66> Setting language to 'eng'
0:00:11.641586594  9671      0x9b04150 WARN               mpegtsmux mpegtsmux.c:869:mpegtsmux_sink_event:<splitmuxsink_rabbitmq_mpegtsmux_4736:sink_66> Setting language to 'eng'
0:00:11.711140623  9671      0x9cd96c0 WARN               mpegtsmux mpegtsmux.c:356:mpegtsmux_pad_reset: releasing pad_data->language
0:00:11.712432191  9671      0x9cd96c0 WARN               mpegtsmux mpegtsmux.c:869:mpegtsmux_sink_event:<splitmuxsink_rabbitmq_mpegtsmux_4287:sink_66> Setting language to 'eng'
0:00:11.736278444  9671      0x9cd96c0 WARN               mpegtsmux mpegtsmux.c:869:mpegtsmux_sink_event:<splitmuxsink_rabbitmq_mpegtsmux_4287:sink_66> Setting language to 'eng'
0:00:12.003196165  9671      0x9ce37b0 WARN               mpegtsmux mpegtsmux.c:356:mpegtsmux_pad_reset: releasing pad_data->language
0:00:12.004533775  9671      0x9ce37b0 WARN               mpegtsmux mpegtsmux.c:869:mpegtsmux_sink_event:<splitmuxsink_rabbitmq_mpegtsmux_4170:sink_66> Setting language to 'eng'
0:00:12.021611624  9671      0x9ce37b0 WARN               mpegtsmux mpegtsmux.c:869:mpegtsmux_sink_event:<splitmuxsink_rabbitmq_mpegtsmux_4170:sink_66> Setting language to 'eng'


We can see that for each invokation of releasing pad_data->language, we have 2 invokations of g_strdup() - which causes memory leak.
Comment 1 Damian Ziobro 2016-04-13 17:30:17 UTC
Created attachment 325875 [details] [review]
Solving memory leak in mpegtsmux by freeing pad_data->language before g_strdup

Attached patch solves this memory leak for me.
Comment 2 Damian Ziobro 2016-04-13 17:47:13 UTC
I reopend this ticket as I don't know whether I should set it to RESOLVED after applying patch. Please let me know.
Comment 3 Tim-Philipp Müller 2016-04-13 17:50:18 UTC
Thanks for the patch. You should not close the bug or change the patch status, a GStreamer developer will do that once they have applied the patch to the GStreamer repository.
Comment 4 Damian Ziobro 2016-04-13 17:58:03 UTC
Created attachment 325878 [details] [review]
Solving memory leak in mpegtsmux by freeing pad_data->language before g_strdup - new
Comment 5 Sebastian Dröge (slomo) 2016-04-14 06:41:47 UTC
Thanks for the patch!

commit 6141cd2f4b2f3ed91b206e5430c0acf666bb800b
Author: Damian Ziobro <damian@xmementoit.com>
Date:   Wed Apr 13 18:12:25 2016 +0100

    mpegtsmux: free pad_data->language before g_strdup in order to avoid memory leak
    
    https://bugzilla.gnome.org/show_bug.cgi?id=765005
Comment 6 Jan Schmidt 2016-04-15 15:53:04 UTC
For future reference, you don't need to check if a variable != NULL before calling g_free. g_free just returns if you pass it NULL, and you don't need to then set the variable to NULL before then assigning a new value from g_strdup :)
Comment 7 Damian Ziobro 2016-04-15 20:18:55 UTC
Hi Jan,
 Thanks for your suggestions. Good to find out it. Actually, I just use if statement as was implemented in mpegtsmux_pad_reset() function. However I will remember for the future.