GNOME Bugzilla – Bug 639976
Personas are never destroyed
Last modified: 2011-02-02 23:19:07 UTC
It seems like Tpf.Persona finalize function is never called. When Tpf.PersonaStore is disposed, the Personas it holds a mapping of have strangely high reference counts.
Yay, another refcount leak. It's most likely in folk's Telepathy backend or the aggregator. I'm fairly sure the Empathy code doesn't leak refcounts.
I started converting all the GLib.Lists to Gee.Containers, because of your general warinesses of the memory management in GLib.Lists. But it ended up being a non-trivial undertaking. Specifically because I don't want to change the API in a way that would expose Gee containers. A curios pattern I saw that looks scary is something like this: var personas = individual.personas.copy (); foreach (var p in personas) p.ref (); We shouldn't need to do that in Vala!
(In reply to comment #2) > I started converting all the GLib.Lists to Gee.Containers, because of your > general warinesses of the memory management in GLib.Lists. But it ended up > being a non-trivial undertaking. Specifically because I don't want to change > the API in a way that would expose Gee containers. Yeah, that's the problem. It might be worth re-evaluating our public API which uses GLists and seeing if perhaps a LinkedHashSet (or some other warm and fuzzy data structure) would be better. Travis? > A curios pattern I saw that looks scary is something like this: > > var personas = individual.personas.copy (); > foreach (var p in personas) > p.ref (); > > We shouldn't need to do that in Vala! My point exactly. :-(
(In reply to comment #3) > (In reply to comment #2) > > I started converting all the GLib.Lists to Gee.Containers, because of your > > general warinesses of the memory management in GLib.Lists. But it ended up > > being a non-trivial undertaking. Specifically because I don't want to change > > the API in a way that would expose Gee containers. > > Yeah, that's the problem. It might be worth re-evaluating our public API which > uses GLists and seeing if perhaps a LinkedHashSet (or some other warm and fuzzy > data structure) would be better. Travis? It's worth considering - I've added bug #640092 as a blocker of this one, with some basic suggestions. Maybe you could check that out, Eitan? We started scrubbing API but haven't quite gotten in all of the planned breaks we had in mind. Better to break sooner rather than later.
Once you have fixed this bug, can you please check if it solves bug #636782 as well?
Add some more goal bugs for the 0.3.5 release.
(Actually setting the new milestone for these bugs)
I tweaked Tpf.PersonaStore to clear out its structures at finalization time (should be equivalent to calling this._reset() in Tpf.~PersonaStore()) and added a bunch of debug statements in Tpf.Persona, Tpf.PersonaStore, and the persona-store-capabilities test (which never involves the Aggregator, simplifying things a bit). At the end of the test case, all the personas have exactly 1 ref (when they should have 0 and be finalized). This extra ref seems to be due to bug #624624. Once this bug is fixed, we shouldn't even have to change our code (assuming all those structures get freed properly at finalization -- which would be a Vala bug if they don't). So, I'll mark this as a dupe. If we turn out to have additional components that cause this specific bug, I'll re-open it. *** This bug has been marked as a duplicate of bug 624624 ***
*** Bug 636782 has been marked as a duplicate of this bug. ***