[gjs/ewlsh/register-type] ci: Ensure forever callbacks do not leak

[Evan Welsh <ewlsh src gnome org>]

commit 852130b276a37634c9162c434885fa4d9ff7879d
Author: Evan Welsh <contact evanwelsh com>
Date:   Thu Dec 30 07:26:07 2021 -0800

    ci: Ensure forever callbacks do not leak

 gi/arg-cache.cpp               | 10 +++++++---
 gi/function.cpp                | 23 ++++++++++++++++++++++-
 gi/function.h                  |  7 +++++++
 gjs/context.cpp                |  2 ++
 installed-tests/js/testGtk4.js |  4 ++--
 5 files changed, 40 insertions(+), 6 deletions(-)
---
diff --git a/gi/arg-cache.cpp b/gi/arg-cache.cpp
index 9fc02613..cf38819a 100644
--- a/gi/arg-cache.cpp
+++ b/gi/arg-cache.cpp
@@ -324,13 +324,17 @@ static bool gjs_marshal_callback_in(JSContext* cx, GjsArgumentCache* self,
     }
 
     GIScopeType scope = self->contents.callback.scope;
-    bool keep_forever = scope == GI_SCOPE_TYPE_NOTIFIED &&
-                        !self->has_callback_destroy();
-    if (trampoline && (scope == GI_SCOPE_TYPE_ASYNC || keep_forever)) {
+    if (trampoline && (scope == GI_SCOPE_TYPE_ASYNC)) {
         // Add an extra reference that will be cleared when garbage collecting
         // async calls or never for notified callbacks without destroy
         g_closure_ref(trampoline);
     }
+
+    bool keep_forever =
+        scope == GI_SCOPE_TYPE_NOTIFIED && !self->has_callback_destroy();
+    if (keep_forever) {
+        trampoline->mark_forever();
+    }
     gjs_arg_set(arg, closure);
 
     return true;
diff --git a/gi/function.cpp b/gi/function.cpp
index acbe3a7b..ebc6cc69 100644
--- a/gi/function.cpp
+++ b/gi/function.cpp
@@ -616,6 +616,9 @@ GjsCallbackTrampoline* GjsCallbackTrampoline::create(
     return trampoline;
 }
 
+decltype(GjsCallbackTrampoline::s_forever_closure_list)
+    GjsCallbackTrampoline::s_forever_closure_list;
+
 GjsCallbackTrampoline::GjsCallbackTrampoline(
     JSContext* cx, JS::HandleFunction function, GICallableInfo* callable_info,
     GIScopeType scope, bool has_scope_object, bool is_vfunc)
@@ -629,7 +632,8 @@ GjsCallbackTrampoline::GjsCallbackTrampoline(
       m_info(callable_info, GjsAutoTakeOwnership()),
       m_param_types(g_callable_info_get_n_args(callable_info), {}),
       m_scope(scope),
-      m_is_vfunc(is_vfunc) {
+      m_is_vfunc(is_vfunc),
+      m_is_forever(false) {
     add_finalize_notifier<GjsCallbackTrampoline>();
 }
 
@@ -638,6 +642,23 @@ GjsCallbackTrampoline::~GjsCallbackTrampoline() {
         g_callable_info_free_closure(m_info, m_closure);
 }
 
+void GjsCallbackTrampoline::mark_forever() {
+    g_closure_ref(this);
+    m_is_forever = true;
+    s_forever_closure_list.push_back(this);
+}
+
+void GjsCallbackTrampoline::prepare_shutdown() {
+    s_forever_closure_list.erase(
+        std::remove_if(s_forever_closure_list.begin(),
+                       s_forever_closure_list.end(),
+                       ([](GjsCallbackTrampoline* trampoline) {
+                           g_closure_unref(trampoline);
+                           return true;
+                       })),
+        s_forever_closure_list.end());
+}
+
 bool GjsCallbackTrampoline::initialize() {
     g_assert(is_valid());
     g_assert(!m_closure);
diff --git a/gi/function.h b/gi/function.h
index a6ca4313..b722cde1 100644
--- a/gi/function.h
+++ b/gi/function.h
@@ -51,6 +51,10 @@ struct GjsCallbackTrampoline : public Gjs::Closure {
 
     constexpr ffi_closure* closure() const { return m_closure; }
 
+    void mark_forever();
+
+    static void prepare_shutdown();
+
  private:
     GJS_JSAPI_RETURN_CONVENTION bool initialize();
     GjsCallbackTrampoline(JSContext* cx, JS::HandleFunction function,
@@ -65,6 +69,8 @@ struct GjsCallbackTrampoline : public Gjs::Closure {
                                 int c_args_offset, void* result);
     void warn_about_illegal_js_callback(const char* when, const char* reason);
 
+    static std::vector<GjsCallbackTrampoline*> s_forever_closure_list;
+
     GjsAutoCallableInfo m_info;
     ffi_closure* m_closure = nullptr;
     std::vector<GjsParamType> m_param_types;
@@ -72,6 +78,7 @@ struct GjsCallbackTrampoline : public Gjs::Closure {
 
     GIScopeType m_scope : 2;
     bool m_is_vfunc : 1;
+    bool m_is_forever : 1;
 };
 
 // Stack allocation only!
diff --git a/gjs/context.cpp b/gjs/context.cpp
index 04736f30..bf7a9e50 100644
--- a/gjs/context.cpp
+++ b/gjs/context.cpp
@@ -61,6 +61,7 @@
 #include <mozilla/UniquePtr.h>
 
 #include "gi/closure.h"  // for Closure::Ptr, Closure
+#include "gi/function.h"
 #include "gi/object.h"
 #include "gi/private.h"
 #include "gi/repo.h"
@@ -438,6 +439,7 @@ void GjsContextPrivate::dispose(void) {
          */
         gjs_debug(GJS_DEBUG_CONTEXT, "Releasing all native objects");
         ObjectInstance::prepare_shutdown();
+        GjsCallbackTrampoline::prepare_shutdown();
 
         gjs_debug(GJS_DEBUG_CONTEXT, "Disabling auto GC");
         if (m_auto_gc_id > 0) {
diff --git a/installed-tests/js/testGtk4.js b/installed-tests/js/testGtk4.js
index d7fc0028..3e6605b1 100644
--- a/installed-tests/js/testGtk4.js
+++ b/installed-tests/js/testGtk4.js
@@ -200,9 +200,9 @@ describe('Gtk 4 regressions', function () {
         expect(() => new Gdk.Event()).toThrowError(/Couldn't find a constructor/);
     });
 
-    xit('Actions added via Gtk.WidgetClass.add_action() should not crash', function () {
+    it('Actions added via Gtk.WidgetClass.add_action() should not crash', function () {
         const custom = new CustomActionWidget();
         custom.activate_action('custom.action', null);
         expect(custom.action).toEqual(42);
-    }).pend('https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/3796');
+    });
 });