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 636804 - Using appsink causes memory leak because buffers are double ref'ed by gst-sharp
Using appsink causes memory leak because buffers are double ref'ed by gst-sharp
Status: RESOLVED DUPLICATE of bug 618336
Product: GStreamer
Classification: Platform
Component: gst-sharp
0.10.x
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2010-12-08 19:40 UTC by William Lahti
Modified: 2010-12-12 22:49 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
test case demonstrating AppSink.PullBuffer memory leak (2.18 KB, text/plain)
2010-12-12 16:51 UTC, William Lahti
Details

Description William Lahti 2010-12-08 19:40:31 UTC
When using the appsink API with gstreamer-sharp to sip buffers via PullBuffer, the underlying buffers are ref'ed one too many times, thus using Dispose() or allowing garbage collection to dispose of a buffer will not cause it to be freed, despite the fact that no actual references for the buffer exist anymore (ie, the reference count of buffers after they are disposed is still 1). 

To confirm this (and also a workaround for affected users), I called gst_mini_object_unref() manually before calling Dispose() and the memory leaking stopped. 

This most likely means that gstreamer-sharp has one extra gst_mini_object_ref call than needed when setting up the C# proxy object.

I plan on submitting a patch soon if I can manage to find the error but for now, a manual call to gst_mini_object_unref before calling dispose (or allowing the object to become garbage collected) is needed.

Just include this pinvoke somewhere:
 
[DllImport ("libgstreamer-0.10.dll") ]
static extern void gst_mini_object_unref (IntPtr raw);

And when you're done with the object you must:

gst_mini_object_unref(buffer.Handle);
Comment 1 Sebastian Dröge (slomo) 2010-12-12 10:10:57 UTC
Could you attach a small testcase to this bugreport? AppSink.PullBuffer() shouldn't increase the refcount of the buffer at all
Comment 2 William Lahti 2010-12-12 16:51:50 UTC
Created attachment 176299 [details]
test case demonstrating AppSink.PullBuffer memory leak

Doesnt gst_appsink_pull_buffer return a referenced buffer, which then need only be unreferenced when the app is finished using it?

I have been looking into the relevant code, and it appears that the generated PullBuffer implementation treats buffers returned by gst_appsink_pull_buffer as unowned when passed to Gst.MiniBuffer.GetObject (it uses the single parameter overload which defaults the second parameter "owned_ref" to false), but I believe they are actually already referenced by gst-app, and need only be unreferenced when the proxy object is disposed. Instead, since they are treated as "unowned" when they are wrapped, Gst.Buffer.GetObject then references them before return. 

The testcase is based on the appsrc sample, and has a switch comment for swapping out appsink for fakesink to see the difference in memory usage. The observed behavior for me with gstreamer-sharp 0.9.2 is a steady memory leak (slow for default videotestsrc buffers, but visible).

It can be compiled with:

gmcs AppSink.cs /r:gstreamer-sharp -pkg:glib-sharp-2.0

Additionally, I have left the gst_minibuffer_unref pinvoke in there, with the call commented out, so you can see how unrefing before dispose fixes the problem.

I believe I have fixed this in my working copy, but just for PullBuffer. Presumably this same problem applies to the generated implementations of PullBufferList et al. If someone can confirm that appsink does in fact leave a ref for the application on buffers then I can go ahead and finish fixing the other PullBuffer variants and post a patch. 

Also, I am fixing by adding the hidden attribute to the gst_appsink_pull_buffer method in GStreamer.metadata file, then adding a custom implementation in AppSink.custom. Is this the correct way to fix this? Alternatively, the GetObject() method in the MiniObject class could be set to default the owned_ref parameter to true, but I don't know if this is appropriate for all it's uses.
Comment 3 Sebastian Dröge (slomo) 2010-12-12 22:49:05 UTC
Ah, I see. This was fixed in GIT already some time ago:

commit 721340c32d93558071e00e2f96e4163f7e98d7aa
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Thu May 13 11:24:22 2010 +0200

    Fix ownership of appsink return values
    
    Patch by kenkela@gmail.com.
    
    Fixes bug #618336.

*** This bug has been marked as a duplicate of bug 618336 ***