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 782719 - Memory leak in deserialize_hash_table()
Memory leak in deserialize_hash_table()
Status: RESOLVED FIXED
Product: vala
Classification: Core
Component: D-Bus
0.37.x
Other Linux
: Normal normal
: ---
Assigned To: Vala maintainers
Vala maintainers
Depends on:
Blocks:
 
 
Reported: 2017-05-17 03:34 UTC by brant.li
Modified: 2017-05-17 20:14 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gdbus: Don't leak nested HashTable on deserialization (5.27 KB, patch)
2017-05-17 07:13 UTC, Rico Tzschichholz
committed Details | Review

Description brant.li 2017-05-17 03:34:00 UTC
Create a dbus proxy object will memory leak If the return type of method is HashTable which is contains another HashTable.
Following simple code reproduces the issues:

Vala code:
>    [DBus (name = "org.freedesktop.DBus.ObjectManager")]
>    public interface DBusObjectManager : Object
>    {
>        public abstract HashTable<ObjectPath, HashTable<string, HashTable<string, Variant>>> get_managed_objects();
>    }

C code:
>    static GHashTable* dbus_object_manager_proxy_get_managed_objects (DBusObjectManager* self) {
>    …
>    g_variant_iter_init (&_tmp2_, _tmp0_);
>    _tmp1_ = g_hash_table_new_full (g_direct_hash, g_direct_equal, NULL, NULL);
>    g_variant_iter_init (&_tmp2_, _tmp0_);
>    while (g_variant_iter_loop (&_tmp2_, "{?*}", &_tmp3_, &_tmp4_)) {
>    	GHashTable* _tmp5_;
>    	GVariantIter _tmp6_;
>    	GVariant* _tmp7_;
>    	GVariant* _tmp8_;
>    	_tmp5_ = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL);
>    	g_variant_iter_init (&_tmp6_, _tmp4_);
>    	while (g_variant_iter_loop (&_tmp6_, "{?*}", &_tmp7_, &_tmp8_)) {
>    		GHashTable* _tmp9_;
>    		GVariantIter _tmp10_;
>    		GVariant* _tmp11_;
>    		GVariant* _tmp12_;
>    		_tmp9_ = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, (GDestroyNotify) g_variant_unref);
>    …

The generated C code did not specify a value destroy function in g_hash_table_new_full if the original HashTable contains another HashTable.

Observing the function of deserialize_hash_table() in codegen/valagvariantmodule.vala,
Create the key/value destroy function for HashTable that only take care of String & Variant type,
would it be possible to add expanded support for other type?
Comment 1 Rico Tzschichholz 2017-05-17 07:13:00 UTC
Created attachment 352003 [details] [review]
gdbus: Don't leak nested HashTable on deserialization

Additionally make sure types derived from string are freed, e.g. ObjectPath