[gjs/ewlsh/fix-forever-callbacks] ci: Prevent forever callbacks from leaking




commit 349a1036983deb4dd9b4e17f2297e48a7fae3540
Author: Evan Welsh <contact evanwelsh com>
Date:   Sun Jan 9 11:23:52 2022 -0800

    ci: Prevent forever callbacks from leaking
    
    GNOME/gjs!647 added initial support for notified callbacks which did not
    expose a destory callback, GNOME/gtk!3796 merged the first usage of
    this support. The current implementation leaks callbacks which breaks CI
    with newer versions of GTK.

 gi/arg-cache.cpp | 14 +++++++++++---
 gi/function.cpp  | 12 ++++++++++++
 gi/function.h    |  6 ++++++
 gjs/context.cpp  |  2 ++
 4 files changed, 31 insertions(+), 3 deletions(-)
---
diff --git a/gi/arg-cache.cpp b/gi/arg-cache.cpp
index 9fc02613..6d1d287c 100644
--- a/gi/arg-cache.cpp
+++ b/gi/arg-cache.cpp
@@ -324,13 +324,21 @@ 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 =
+#if GI_CHECK_VERSION(1, 72, 0)
+        scope == GI_SCOPE_TYPE_FOREVER ||
+#endif
+        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..d2263635 100644
--- a/gi/function.cpp
+++ b/gi/function.cpp
@@ -7,6 +7,7 @@
 #include <stdint.h>
 #include <stdlib.h>  // for exit
 
+#include <algorithm>  // for remove_if
 #include <memory>  // for unique_ptr
 #include <string>
 #include <type_traits>
@@ -616,6 +617,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)
@@ -638,6 +642,14 @@ GjsCallbackTrampoline::~GjsCallbackTrampoline() {
         g_callable_info_free_closure(m_info, m_closure);
 }
 
+void GjsCallbackTrampoline::mark_forever() {
+    s_forever_closure_list.emplace_back(this, GjsAutoTakeOwnership{});
+}
+
+void GjsCallbackTrampoline::prepare_shutdown() {
+    s_forever_closure_list.clear();
+}
+
 bool GjsCallbackTrampoline::initialize() {
     g_assert(is_valid());
     g_assert(!m_closure);
diff --git a/gi/function.h b/gi/function.h
index a6ca4313..5ebe9b89 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<GjsAutoGClosure> s_forever_closure_list;
+
     GjsAutoCallableInfo m_info;
     ffi_closure* m_closure = nullptr;
     std::vector<GjsParamType> m_param_types;
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) {


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]