Leaking widgets

[bleakgrey]

Describe the bug

All Status widgets refuse to get destroyed. As a side-effect, timeline-based pages like Profile or Conversations don't get destroyed too, and entity cache keeps the statuses and images they contained in memory forever.

I'm pretty sure it didn't happen in Tootle (can't tell for sure though, it didn't compile on my device), which means this is caused by the new functionality in the Status widget.

I couldn't pinpoint the exact cause yet - this issue needs testing.

Steps To Reproduce

1. Open many timelines in the main window
2. Navigate back
3. Observe the terminal output - there are no messages like Destroying Status widget

Logs and/or Screenshots

No response

Instance Backend

Mastodon

Operating System

Pop!_OS 22.04 LTS

Package

Flatpak

Troubleshooting information

flatpak: true version: 2.1.0 gtk: 4.8.3 (4.8.3) libadwaita: 1.2.0 (1.2.0)

Additional Context

When destroyed properly, Status widgets emit a debug message specified in Status.vala (line 139). Additionally, unused API.Status entities get freed by entity cache with a message specified in AbstractCache.vala (line 39).

All of these aren't present in main.

[bleakgrey]

Update: I just noticed that Views.Lists is leaking too - there is no proper destruction message in the terminal.

[GeopJr]

can confirm they get destroyed correctly on Tootle:

** Message: 20:28:07.795: Status.vala:76: Destroying Status widget
** Message: 20:28:07.797: Thread.vala:18: Destroying Thread
** Message: 20:28:07.797: ContentBase.vala:32: Destroying ContentBase
** Message: 20:28:07.797: Base.vala:53: Destroying base Conversation

** Message: 20:28:23.278: ContentBase.vala:32: Destroying ContentBase
** Message: 20:28:23.278: Base.vala:53: Destroying base Lists

My current speculation is that I probably messed it up during the navigation rethink commits:

https://github.com/GeopJr/Tooth/commit/5ba32dfa8357426f671c7183455401d399e44f40 & https://github.com/GeopJr/Tooth/commit/c73927b08d1cb95df8ca660b976a738179d498f1

[bleakgrey]

Both commits seem harmless enough to me, it can't be navigation. I'd check if new property bindings or something like that was added to the Status and Lists widgets themselves.

[GeopJr]

Narrowed it down to 6999d465390e202a7f72cdbd14bbd4943cdf4e07! (for status)

[bleakgrey]

Does commenting out the bindings solve it?

[GeopJr]

For that commit, yes! For HEAD however not (probably because we added more).

[bleakgrey]

That's a good start! Perhaps this is why some things were not reactive in Tootle 😅

[GeopJr]

I'm slightly confused - it appears to occur only when transformers are used:

Status gets destroyed:

status.formal.bind_property ("favourites-count", fav_count_label, "label", BindingFlags.SYNC_CREATE);

Status does not get destroyed:

status.formal.bind_property ("favourites-count", fav_count_label, "label", BindingFlags.SYNC_CREATE, (b, src, ref target) => {
            int64 srcval = (int64) src;
            target.set_string (@"<b>$srcval</b> " + _("Favourites"));
            return true;
        });

At the same time creating properties instead works just fine :/ (status gets destroyed):

public int64 favourites {
        set {
            fav_count_label.label = @"<b>$((int64) value)</b> " + _("Favourites");
        }
    }

status.formal.bind_property ("favourites-count", this, "favourites", BindingFlags.SYNC_CREATE);

[bleakgrey]

I may have an idea. What if we try using GLib.BindingGroup instead of individual bindings? For instance:

status.formal.bind_property ("account", avatar, "account", BindingFlags.SYNC_CREATE);
...

will turn into

var formal_bindings = new GLib.BindingGroup ();
formal_bindings.bind_property ("account", avatar, "account", BindingFlags.SYNC_CREATE);
...
formal_bindings.set_source (status.formal);

and

bind_property ("reveal-spoiler", spoiler_stack, "visible-child-name", BindingFlags.SYNC_CREATE, (b, src, ref target) => {
        var name = reveal_spoiler ? "content" : "spoiler";
        spoiler_status_con.visible = src.get_boolean() && status.formal.has_spoiler;
        target.set_string (name);
        return true;
});
...

into

var self_bindings = new BindingGroup ();
self_bindings.bind_property ("reveal-spoiler", spoiler_stack, "visible-child-name", BindingFlags.SYNC_CREATE, (b, src, ref target) => {
            var name = reveal_spoiler ? "content" : "spoiler";
            spoiler_status_con.visible = src.get_boolean() && status.formal.has_spoiler;
            target.set_string (name);
            return true;
});
...
self_bindings.set_source (this);

[GeopJr]

I'll give it a try in a second, but here's my "create property instead of using transformers" try (which fixes status destruction on HEAD):

fix-status-destruction.patch.txt

[GeopJr]

Sorry for the wait, here's the patch with BindingGroups:

fix-status-destruction-bindinggroup.patch.txt

While it does destroy the widget, spoilers seem to have stopped working (clicking them does nothing)

[GeopJr]

By the way, is this expected behavior with transformers or should I report it to the Vala team?

[bleakgrey]

I'd say this feels like a bug, so it probably should be reported.

[GeopJr]

Apart from lists, profile view would also leak. It always has to do with connecting some signals to anonymous functions :/ At least it's now easier to recognize