Skip to content

Fix App memory leak - #608

Open
lenemter wants to merge 1 commit into
mainfrom
lenemter/fix-app-memory-leak
Open

Fix App memory leak#608
lenemter wants to merge 1 commit into
mainfrom
lenemter/fix-app-memory-leak

Conversation

@lenemter

@lenemter lenemter commented Aug 22, 2026

Copy link
Copy Markdown
Member

The issue was in GLib.HashTable.steal_extended use. The whole point of steal* methods is to transfer ownership of the objects without calling destroy functions. But since the second argument (stolen key) was null, the ownership was never transferred and destroy function (unref if this case) was never called, which lead to memory leak.

Instead of dealing with convoluted method, just use @get and remove.

@lenemter
lenemter requested a review from a team August 22, 2026 17:45

@leolost2605 leolost2605 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we use GLib.HashTable.take ()? Would make it just a little bit more readable IMO

@lenemter

Copy link
Copy Markdown
Member Author

@leolost2605 GLib.HashTable.take () is basically GLib.HashTable.steal () under the hood:

public V? take (K key, out bool exists = null) {
	GLib.HashTable<K,V>? ht = null;
	void** htp = &ht;
	*htp = this.lookup (key);
	exists = this.steal (key);
	return ht;
}

GLib.HashTable.steal () "removes a key and its associated value from a GLib.HashTable without calling the key and value destroy functions", so basically the same issue as in GLib.HashTable.steal_extended

@leolost2605 leolost2605 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Too bad, I thought since it's a vala method take would take care of the key or just do it differently altogether but ig not :(

https://gitlab.gnome.org/GNOME/vala/-/work_items/1664

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants