GNOME Bugzilla – Bug 608702
Memory leak when wrapping GstMiniObject derived C objects
Last modified: 2012-09-12 03:35:09 UTC
Created attachment 152745 [details] Valgrind log with Gst::Message::wrap and Gst::Query::wrap leak Version 0.10.6 I was debugging an application of mine, and saw a couple of leaks. Tried compiling the oggplayer example app and ran it under valgrind. Attached you'll find the log (please disregard oggdemux leaks, I've reported it in a different bug against gstreamer). It seems that Gst::Message and Gst::Query are leaking. Please note that I used gnome sonar.ogg file for testing, which is less than a second long. The command used for generating the report was: G_SLICE=always-malloc G_DEBUG=gc-friendly GLIBCPP_FORCE_NEW=1 GLIBCXX_FORCE_NEW=1 valgrind --tool=memcheck --leak-check=full --leak-resolution=high --trace-children=yes --num-callers=20 --log-file=testrun.txt --suppressions=/home/negro/tmp/gst.supp -v ./a.out /usr/share/sounds/gnome/default/alerts/sonar.ogg
The problem is that wrap methods specific to some Gst::MiniObject derived classes (namely Gst::Event, Gst::Message and Gst::Query) had to be defined so that it would be possible to convert the C API's general types (GstEvent, GstMessage, etc.) to more specific C++ classes which could be used in a C++ object oriented fashion. Unfortunately, these specific methods create new instances of the appropriate more specific C++ classes, wrapped in Glib::RefPtr<>'s but those new instances are never freed because GstMiniObject does not have a destroy notification mechanism (some other way must be found). From your valgrind output, it is evident that some callbacks (employed as "glue" for the slot types that gstreamermm uses) call the classes' specific wrap methods to wrap the C types before sending them off to the user defined slots. The new instances are not freed thus the leak you're reporting. In the callbacks I think it's possible to free the instances after they are used and that would "plug" the leaks. However, signals and virtual functions with these Gst::MiniObject derived classes in their parameter list and using these class specific wrap methods would most likely also exhibit this behavior. A sure way to make sure there is no leak would be to also handwrite wrapped signals and virtual function with these Gst::MiniObject derived classes in their parameter list, but that just doesn't sound like a good way to solve this. I'm hoping there are other (better) solutions.
The strange part is that those objects don't seem to leak every time they're used... I don't have enough knowledge and skill to trak the problem down myself, but I hope the example code I used (oggdec/main.cc) is simple enough to shed some light on the issue at hand...
If the objects are not wrapped with the wrap() methods of the derived classes those objects wont leak. The leaking will probably occur when the C objects are wrapped using the wrap() methods. One possible solution might be to have a small Gst::MiniObject specific RefPtr class (Gst::MiniRefPtr<>) which deletes the wrapper when it is destroyed. Then the derived classes' wrap() methods could return that "Gst::MiniRefPtr<>" which would automatically get rid of the wrapper after it is used (the leak would be plugged). Another possible solution would be to have the wrap() methods of the derived classes return a std::auto_ptr<> which would achieve a similar result as the custom refptr. While these options are explored, this report can remain open. Thanks.
Please let me know if I can help with testing.
This is more complicated than what it seemed like initially. The MiniRefPtr<> idea is not practical because it is hard to predict what happens to the wrappers in the refptr's and how they are manipulated when they are passed or assigned to. Having the derived classes' wrap() methods would not be usable either for the same reasons as the MiniRefPtr<> idea. The one idea that I could come up with is to somehow store the wrappers in the mini objects and hook into the finalize virtual function of the GstMiniObject class to then see if there is a wrapper which could then be destroyed on finalization. For derived classes like GstMessage, GstEvent and GstQuery this is possible because they each have a GstStructure in which it is possible to store the wrapper. The following commit modifies Gst::Message to exhibit this new functionality and exemplifies how it could be done for the rest of the classes with GstStructures: http://git.gnome.org/browse/gstreamermm/commit/?id=043fe17814f21fe05aae9af7146b8baffd90a172 The one class left (GstBuffer) has no structure and fixing a possible leak with this class is a bit more difficult. The theory is the same, but some way would have to be found to keep track of the wrappers. One (very bad) way would be to use an unused member (defined for ABI purposes) in the GstMiniObject instance to store the wrappers on a "per-instance" basis which could then be deleted on finalization as with the rest of the GstMiniObject derived classes (I've tested this and as ugly as it may be it does work). The other way would be to use some sort of map in which the wrappers can be stored (for Gst::Buffer only) and delete them on finalization from the "global" map. For now, at least three out of the four MiniObject derived classes can be fixed. The fourth will have to wait until the other maintainers make comments on which might be the best course of action to take. I'd ask you to test, but it would have to be from git: http://git.gnome.org/browse/gstreamermm/ Please feel free to do so if you'd like.
I didn't clarify that the leaking would be exhibited in any GstMiniObject derived C object that is wrapped, not just in the class specific wrap methods. Also, using a private member in the GstMiniObject instance seems too unpredictable. The idea no longer seems feasible.
I don't think this can be fixed without GstMiniObject supporting some sort of basic storage of data for bindings because storing things in the GstStructure of the MiniObject classes is not always possible due to the structure not being writable depending on the referencing of the owner so I filed an enhancement request with the GStreamer developers: https://bugzilla.gnome.org/show_bug.cgi?id=609473 I'm not sure it will be approved, but at least it is in their consideration.
Created attachment 187475 [details] [review] Patch to ensure GstMiniObject's new weak referencing would fix this bug. There's a new patch adding weak referencing functionality to GstMiniObject that would allow a fix for this bug. This small patch to Gst::Message and Gst::MiniObject confirms that the new functionality can be used to avoid the leak (just run the tests/test-message-wrap test after applying both patches and rebuilding). An output statement is added to the callback but once the statement is removed, freeing the custom created wrapper will occur seamlessly without the user being made aware of what's taking place.
It took a long time, but finally this bug has been fixed in git by this commit: http://git.gnome.org/browse/gstreamermm/commit/?id=3a83d2215253a3099c1c521c84123bfed7849855 Sorry for such a delay. Feel free to reopen if the bug persists.