Surprise: all GtkChild widgets must be declared as unowned
Traditionally, Vala applications declare composite template widget children like this (example from Chess):
[GtkChild]
private Gtk.Box? button_box;
[GtkChild]
private Gtk.Image queen_image;
[GtkChild]
private Gtk.Image knight_image;
[GtkChild]
private Gtk.Image rook_image;
[GtkChild]
private Gtk.Image bishop_image;
But this is wrong, because Vala does not take any refs on these widgets: only GTK has a strong ref on the widget children. For correctness, they must be declared like this:
[GtkChild]
private unowned Gtk.Box? button_box;
[GtkChild]
private unowned Gtk.Image queen_image;
[GtkChild]
private unowned Gtk.Image knight_image;
[GtkChild]
private unowned Gtk.Image rook_image;
[GtkChild]
private unowned Gtk.Image bishop_image;
But, checking codesearch.debian.net, no applications do this.
Why did we not notice sooner? Because, as documented by gtk_widget_class_bind_template_child_full(), GTK drops its references to the widgets in dispose, and also sets them to NULL. Then Vala unrefs the child widgets in finalize, even though it doesn't have any refs. Luckily, we avoid crashing because Vala uses _g_object_unref0, and GTK has already set the widgets to NULL in dispose, so the unrefs never happen.
Possible solutions:
- Treat all GtkChild widgets as implicitly unowned (this would be my preference, if possible). Optionally, print a warning if redundantly declared as unowned
- Document that all GtkChild widgets must be declared as unowned, and print a warning if not (but if Vala can do this, it could probably just make them all implicitly unowned?)
Note that taking a ref on the widgets will not work, because GTK will NULL out the widgets before Vala drops its refs in finalize.
Additional context:
In practice, this has thus far caused no problems, because for Vala to improperly unref the widget with _g_object_unref0 before GTK sets it to null, an app would need to manually set it to null before its destructor runs. And in GTK 3, you really have no reason to ever do that. But in GTK 4, you have to unparent your direct child widgets in dispose. (Since Vala probably does not know which child widgets are direct children, I guess it's probably best for app developers to do this manually, rather than have Vala do so automatically... although it would be amazing sugar if Vala could do that itself!) In C you would do this by writing g_clear_pointer (&self->widget, gtk_widget_unparent)
. The most intuitive way to replicate this in Vala for a template widget with one direct child is:
public override void dispose ()
{
if (widget != null)
{
widget.unparent ();
widget = null;
}
base.dispose ();
}
But that will crash if the widget is not declared as unowned (because Vala will unref widget immediately before setting it to null).
One workaround would be:
public override void dispose ()
{
if (widget != null)
widget.unparent ();
base.dispose ();
}
I think this is safe because GTK will always set the widget to NULL itself on the very first call to dispose. Even if it didn't do that, it would still be safe because unparent does nothing if called multiple times. But this version is weird. It is mandatory to check that the widget is not null before using it, because dispose could be called multiple times, but nowhere does the programmer explicitly set it to null, so it looks like the check is unnecessary. Hence, I don't like this version.
CC @otte