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




commit a41de4783d5deebbc977ef15b9b7c714c75eaa7d
Author: Evan Welsh <contact evanwelsh com>
Date:   Thu Dec 30 07:37:46 2021 -0800

    ci: Ensure forever callbacks do not leak

 gi/arg-cache.cpp                  | 10 +++++++---
 gi/function.cpp                   | 24 +++++++++++++++++++++++-
 gi/function.h                     |  7 +++++++
 gjs/context.cpp                   |  2 ++
 modules/core/overrides/GObject.js |  2 --
 5 files changed, 39 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..a5aa11c1 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)
@@ -629,7 +633,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 +643,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/modules/core/overrides/GObject.js b/modules/core/overrides/GObject.js
index 92f01911..96428f0b 100644
--- a/modules/core/overrides/GObject.js
+++ b/modules/core/overrides/GObject.js
@@ -713,8 +713,6 @@ function _init() {
         _wrapInterface(this);
     };
 
-
-
     GObject.Object.prototype.bindPropertyFields = bindPropertyFields;
 
     GObject.Interface._classInit = function (klass) {


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