From d473efcbfe3603fac62594bd9d8fee8b23ffcbe5 Mon Sep 17 00:00:00 2001
From: Christopher Vollick <0@psycoti.ca>
Date: Thu, 14 Sep 2023 15:12:58 -0400
Subject: Add WeakTimeout Pattern to Prevent Leaks

While doing testing I noticed that skeletons were being leaked, and
eventually tracked it down to the timer that updates the time label
closing over "this" and then keeping the reference alive, potentially
for 24 hours.

I noticed a few other places in the code doing some version of this, and
one of them had the "static and weak pointer" approach, which I pulled
out into a util class. Now, we still have to make sure we're passing it
a static method instead of a lambda, as that would also close over
"this" and render the whole thing useless, but at least most of the
annoying parts live in the util class now.

Also the call_widget version was doing a weird thing where it was
removing itself, but then returning "true"? I'm not sure what that
accomplishes, because returning "false" means to not run this again. So
I think my new version is the same in practice, but simpler...

There are other timeouts in the code that I briefly looked over, but all
of them seemed to be relatively short hard-coded durations, so I left
them alone.

But if any of them are long-lived, it's possible they could also benefit
from this class in the future.

Closes #1480

Co-Authored-By: Marvin W <git@larma.de>
---
 libdino/src/util/weak_timeout.vala | 40 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 libdino/src/util/weak_timeout.vala

(limited to 'libdino/src/util')

diff --git a/libdino/src/util/weak_timeout.vala b/libdino/src/util/weak_timeout.vala
new file mode 100644
index 00000000..28894ed3
--- /dev/null
+++ b/libdino/src/util/weak_timeout.vala
@@ -0,0 +1,40 @@
+public class Dino.WeakTimeout {
+    // XXX: If you get an error saying your function doesn't match the delegate, make sure it's static!
+    //      These are marked as "has_target=false" so you can't close over "this" and leak it in your lambda.
+    [CCode (has_target = false, instance_pos = 0)]
+    public delegate bool SourceFunc<T> (T object);
+
+    [CCode (has_target = false, instance_pos = 0)]
+    public delegate void SourceOnceFunc<T> (T object);
+
+    public static uint add<T>(uint interval, T object, owned SourceFunc<T> function, int priority = GLib.Priority.DEFAULT) {
+        var weak = WeakRef((Object)object);
+        return GLib.Timeout.add(interval, () => {
+            var strong = weak.get();
+            if (strong == null) return false;
+
+            return function(strong);
+        }, priority);
+    }
+
+    public static uint add_once<T>(uint interval, T object, owned SourceOnceFunc<T> function, int priority = GLib.Priority.DEFAULT) {
+        var weak = WeakRef((Object)object);
+        return GLib.Timeout.add(interval, () => {
+            var strong = weak.get();
+            if (strong == null) return false;
+
+            function(strong);
+            return false;
+        }, priority);
+    }
+
+    public static uint add_seconds<T>(uint interval, T object, owned SourceFunc<T> function, int priority = GLib.Priority.DEFAULT) {
+        return add(interval * 1000, object, (owned) function, priority);
+    }
+
+    // This one doesn't have an upstream equivalent, but it seems pretty obvious to me
+    public static uint add_seconds_once<T>(uint interval, T object, owned SourceOnceFunc<T> function, int priority = GLib.Priority.DEFAULT) {
+        return add_once(interval * 1000, object, (owned) function, priority);
+    }
+
+}
-- 
cgit v1.2.3-70-g09d2