Re: [Vala] refcounting, signals and closures

[Michael Gratton <mike vee net>]

On Thu, Aug 11, 2016 at 12:10 PM, Michael Gratton <mike vee net> wrote:
> I'm trying to track down a memory leak in Geary. I have a custom 
> subclass of Gtk.ListBox, instances of which are frequently added and 
> removed from the UI.
[snip]
> The only things I can find that could still be holding a reference 
> are registered signal handlers, both to instance methods, eg:
>
> > this.row_activated.connect(on_row_activated);
>
> And using closures/lambdas:
>
> > this.size_allocate.connect(() => { check_mark_read(); });

So it turns out that the problem was neither of the above. Rather, it 
was due to instance methods being used as higher-order functions by 
being passed as arguments to methods.

For example:

> public class Example : Gtk.ListBox {
>
>     public Example() {
>         set_sort_func(on_sort);
>     }
>
>     private int on_sort(Gtk.ListBoxRow a, Gtk.ListBoxRow b) {
>         return mycmp(a, b);
>     }
>
> }


This widget will never be finalised, even if ::destroy() is called on 
it. To ensure it is finalised, I need to override ::destroy() to unset 
the sort func, then ensure ::destroy() is called:

>     private void destroy() {
>         set_sort_func(null);
>         base.destroy();
>     }

My best guess about what is happening here is that that the closure 
created when calling set_sort_func(on_sort) is adding a ref to the 
object instance, hence when the widget refs the closure in 
set_sort_func() a circular ref is created.

I'd argue that GTK is probably correct in not unsetting the sort func 
on destroy, for the same reason it doesn't also unset other properties 
set by the app.

So is this then a Vala bug? Should a closure created for calling 
instance methods in this way use a weak reference to the instance to 
avoid exactly this situation? In the case above it is very problematic, 
but perhaps if an instance method is passed to an external 
function/method like Timeout.add_seconds_full() then that should hold a 
non-weak-ref? Perhaps the rule should be "Closures should use a weak 
reference to `this` if they are created to be passed to a method on 
`this`, but that sounds a bit sketchy, too - what if that method passes 
it elsewhere?

//Mike

--
⊨ Michael Gratton, Percept Wrangler.
⚙ <http://mjog.vee.net/>