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 615572 - Buffer Leak in audiorate during fill process
Buffer Leak in audiorate during fill process
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal normal
: 0.10.29
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2010-04-12 20:25 UTC by Benjamin Bazso
Modified: 2010-04-16 18:52 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
The patch that fixes the leak mentioned in this issue. (437 bytes, patch)
2010-04-12 20:25 UTC, Benjamin Bazso
rejected Details | Review
audiorate: Don't leak the input buffer in error cases (1.11 KB, patch)
2010-04-16 18:04 UTC, Sebastian Dröge (slomo)
none Details | Review
audiorate: Don't leak the input buffer in error cases (1.36 KB, patch)
2010-04-16 18:06 UTC, Sebastian Dröge (slomo)
committed Details | Review

Description Benjamin Bazso 2010-04-12 20:25:46 UTC
Created attachment 158532 [details] [review]
The patch that fixes the leak mentioned in this issue.

Overview: 

A buffer leak (Memory leak) was detected in the audiorate module when performing  a fill operation. 

When the fill buffer was not successfully pushed (i.e. ret != GST_FLOW_OK) the input buffer is not being unreffed; hence, causing the memory leak mentioned above.

Steps to Reproduce:

In my case, I was stopping the pipeline a random intervals which the pipeline was processing.  This produced the aforementioned leak.

Actual Results: 

The application leaked memory.

Expected Results: 

The application should not leak.

Build Date & Platform:

    git

Additional Information: 

Attached is the patch that fixes this issue.
Comment 1 Tim-Philipp Müller 2010-04-12 20:44:31 UTC
Comment on attachment 158532 [details] [review]
The patch that fixes the leak mentioned in this issue.

This doesn't look right at all. gst_pad_push() always takes ownership of the buffer (reference), so unreffing depending on the flow return is definitely wrong. Either the leak is elsewhere in audiorate, or a downstream element doesn't unref the buffer correctly in the case you describe. What elements are downstream of audiorate in your case?

Maybe you could add a unit test to tests/check/elements/audiorate.c that demonstrates the problem?

Have you tried newer versions of gst-plugins-base? 0.10.20 is a bit old.
Comment 2 Benjamin Bazso 2010-04-13 16:22:12 UTC
I understand that gst_pad_push() always takes ownership of the
buffer (reference), but the buffer being pushed in the case of the fill operation is the fill buffer:

i.e. ret = gst_pad_push (audiorate->srcpad, fill); 

not the input buffer to the chain function: 

gst_audio_rate_chain (GstPad * pad, GstBuffer * buf)

and it is the input buffer that leaks in the issue I described since it never gets freed (see the jump to beach).

If this does not make sense, or I made a mistake, then let me know and I can always try and put together a unit test or something to demonstrate the problem.
Comment 3 Tim-Philipp Müller 2010-04-13 16:35:07 UTC
Ah, my apologies, looks like I misread the patch.
Comment 4 Sebastian Dröge (slomo) 2010-04-16 18:04:01 UTC
Created attachment 158909 [details] [review]
audiorate: Don't leak the input buffer in error cases

Fixes bug #615572.
Comment 5 Sebastian Dröge (slomo) 2010-04-16 18:06:04 UTC
Created attachment 158910 [details] [review]
audiorate: Don't leak the input buffer in error cases

Fixes bug #615572.
Comment 6 Sebastian Dröge (slomo) 2010-04-16 18:07:22 UTC
(In reply to comment #5)
> Created an attachment (id=158910) [details] [review]
> audiorate: Don't leak the input buffer in error cases
> 
> Fixes bug #615572.

This patch fixes the leak in a different way... there were more places where the buffer would be leaked. Tim, shall I push this for next pre-release?
Comment 7 Sebastian Dröge (slomo) 2010-04-16 18:52:35 UTC
commit 0a8b8ceda0ee88e6c2b85563a7c1315e4e12b0a6
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Fri Apr 16 20:03:21 2010 +0200

    audiorate: Don't leak the input buffer in error cases
    
    Fixes bug #615572.