GNOME Bugzilla – Bug 702943
Vala unnecesarily refs values returned from functions marked as transfer-floating
Last modified: 2014-03-30 22:51:40 UTC
Created attachment 247596 [details] Test case (Vala) I've noticed that vala always generates call to g_object_ref_sink(), even if function's return value is marked as transfer-floating. This can lead to memory leaks. Good example of such behaviour is Gst.GhostPad from gstreamer-1.0 bindings. Please take a look at attached code and compare it with code sample from GStreamer docs: http://gstreamer.freedesktop.org/data/doc/gstreamer/head/manual/html/section-pads-ghost.html Moreover, while debugging my app (that uses Ghost Pads extensively) valgrind has pointed that memory is definitely lost and it was always pointing to line similar to _tmp5_ = (GstGhostPad*) gst_ghost_pad_new ("sink", _tmp4_); from the attached file. Manually unreffing ghost pad right after creation removes valgrind's warning and app indeed, stops eating memory. I use vala 0.20.1
Created attachment 247597 [details] Test case (C)
ref_sink is _exactly_for floating return values. Are you running valgrind with the usual glib-specific flags?
Yes, I run valgrind with this command: G_SLICE=always-malloc G_DEBUG=gc-friendly valgrind --tool=memcheck --leak-check=full --leak-resolution=high --num-callers=20 COMMAND I know that g_object_ref_sink() is for floating objects but that is not the case. GStreamer's gst_element_add_pad() expects to receive ghost pad with floating = true. If you call g_object_ref_sink(), you remove floating flag and subsequent calls unnecesarily increase ref count. The outcome is that such pad is not freed when parent object finalizes. Please take a look here: http://cgit.freedesktop.org/gstreamer/gstreamer/tree/gst/gstelement.c?id=1.0.6#n654 and then here: http://cgit.freedesktop.org/gstreamer/gstreamer/tree/gst/gstobject.c?id=1.0.6#n691 GStreamer already internally calls g_object_ref_sink() in gst_object_set_parent(). It should not be present in code generated by vala. It is indicated in GStreamer docs by transfer-floating tag, see http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gstreamer/html/GstGhostPad.html#gst-ghost-pad-new
Then add_pad parameter must be unowned, not owned. Looks like the annotation of add_pad is wrong: http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gstreamer/html/GstElement.html#gst-element-add-pad It says transfer-full, but if it adds a ref, it shouldn't be full.
Should it be fixed in vala bindings or should I file another bug in gstreamer's bugzilla?
In my opinion the problem is in gstreamer. Given it ref's the parameter, the transfer should be none rather than full.
I've created Bug 702960 in GStreamer