function: Use c++ features to handle GjsCallbackTrampoline

Make the struct a bit smarter, including auto pointers and methods to
access to the internal values.

Still manage the object creation and reference counting externally.

4 files changed, 137 insertions, 108 deletions

diff --git a/gi/arg-cache.cpp b/gi/arg-cache.cpp
index ac356abb..9107deaa 100644
--- a/gi/arg-cache.cpp
+++ b/gi/arg-cache.cpp
@@ -316,9 +316,9 @@ static bool gjs_marshal_callback_in(JSContext* cx, GjsArgumentCache* self,
                 return false;
             }
 
-            priv->associate_closure(cx, trampoline->js_function);
+            priv->associate_closure(cx, trampoline->js_function());
         }
-        closure = trampoline->closure;
+        closure = trampoline->closure();
     }
 
     if (self->has_callback_destroy()) {
diff --git a/gi/function.cpp b/gi/function.cpp
index 5adacc62..82fa810d 100644
--- a/gi/function.cpp
+++ b/gi/function.cpp
@@ -28,7 +28,6 @@
 #include <string.h>  // for strcmp, memset, size_t
 
 #include <memory>  // for unique_ptr
-#include <new>
 #include <string>
 #include <type_traits>
 #include <vector>
@@ -102,14 +101,8 @@ GjsCallbackTrampoline* gjs_callback_trampoline_ref(
 void
 gjs_callback_trampoline_unref(GjsCallbackTrampoline *trampoline)
 {
-    if (g_atomic_ref_count_dec(&trampoline->ref_count)) {
-        g_clear_pointer(&trampoline->js_function, g_closure_unref);
-        if (trampoline->info && trampoline->closure)
-            g_callable_info_free_closure(trampoline->info, trampoline->closure);
-        g_clear_pointer(&trampoline->info, g_base_info_unref);
-        g_free (trampoline->param_types);
-        g_free(trampoline);
-    }
+    if (g_atomic_ref_count_dec(&trampoline->ref_count))
+        delete trampoline;
 }
 
 template <typename T, GITypeTag TAG = GI_TYPE_TAG_VOID>
@@ -210,20 +203,14 @@ set_return_ffi_arg_from_giargument (GITypeInfo  *ret_type,
     }
 }
 
-static void
-warn_about_illegal_js_callback(const GjsCallbackTrampoline *trampoline,
-                               const char *when,
-                               const char *reason)
-{
+void GjsCallbackTrampoline::warn_about_illegal_js_callback(const char* when,
+                                                           const char* reason) {
     g_critical("Attempting to run a JS callback %s. This is most likely caused "
                "by %s. Because it would crash the application, it has been "
                "blocked.", when, reason);
-    if (trampoline->info) {
-        const char *name = g_base_info_get_name(trampoline->info);
-        g_critical("The offending callback was %s()%s.", name,
-                   trampoline->is_vfunc ? ", a vfunc" : "");
-    }
-    return;
+    if (m_info)
+        g_critical("The offending callback was %s()%s.", m_info.name(),
+                   m_is_vfunc ? ", a vfunc" : "");
 }
 
 /* This is our main entry point for ffi_closure callbacks.
@@ -233,30 +220,26 @@ warn_about_illegal_js_callback(const GjsCallbackTrampoline *trampoline,
  * In other words, everything we need to call the JS function and
  * getting the return value back.
  */
-static void gjs_callback_closure(ffi_cif* cif [[maybe_unused]], void* result,
-                                 void** ffi_args, void* data) {
+void GjsCallbackTrampoline::callback_closure(GIArgument** args, void* result) {
     JSContext *context;
     int i, n_args, n_jsargs, n_outargs, c_args_offset = 0;
     GITypeInfo ret_type;
     bool success = false;
-    auto args = reinterpret_cast<GIArgument **>(ffi_args);
-
-    g_assert(data && "Trampoline data is not set");
-    GjsAutoCallbackTrampoline trampoline(
-        static_cast<GjsCallbackTrampoline*>(data), GjsAutoTakeOwnership());
 
-    if (G_UNLIKELY(!gjs_closure_is_valid(trampoline->js_function))) {
-        warn_about_illegal_js_callback(trampoline, "during shutdown",
+    if (G_UNLIKELY(!gjs_closure_is_valid(m_js_function))) {
+        warn_about_illegal_js_callback(
+            "during shutdown",
             "destroying a Clutter actor or GTK widget with ::destroy signal "
             "connected, or using the destroy(), dispose(), or remove() vfuncs");
         gjs_dumpstack();
         return;
     }
 
-    context = gjs_closure_get_context(trampoline->js_function);
+    context = gjs_closure_get_context(m_js_function);
     GjsContextPrivate* gjs = GjsContextPrivate::from_cx(context);
     if (G_UNLIKELY(gjs->sweeping())) {
-        warn_about_illegal_js_callback(trampoline, "during garbage collection",
+        warn_about_illegal_js_callback(
+            "during garbage collection",
             "destroying a Clutter actor or GTK widget with ::destroy signal "
             "connected, or using the destroy(), dispose(), or remove() vfuncs");
         gjs_dumpstack();
@@ -264,21 +247,21 @@ static void gjs_callback_closure(ffi_cif* cif [[maybe_unused]], void* result,
     }
 
     if (G_UNLIKELY(!gjs->is_owner_thread())) {
-        warn_about_illegal_js_callback(trampoline, "on a different thread",
-            "an API not intended to be used in JS");
+        warn_about_illegal_js_callback("on a different thread",
+                                       "an API not intended to be used in JS");
         return;
     }
 
-    JSAutoRealm ar(context, JS_GetFunctionObject(gjs_closure_get_callable(
-                                trampoline->js_function)));
+    JSAutoRealm ar(
+        context, JS_GetFunctionObject(gjs_closure_get_callable(m_js_function)));
 
-    bool can_throw_gerror = g_callable_info_can_throw_gerror(trampoline->info);
-    n_args = g_callable_info_get_n_args(trampoline->info);
+    bool can_throw_gerror = g_callable_info_can_throw_gerror(m_info);
+    n_args = m_param_types.size();
 
     g_assert(n_args >= 0);
 
     JS::RootedObject this_object(context);
-    if (trampoline->is_vfunc) {
+    if (m_is_vfunc) {
         GObject* gobj = G_OBJECT(gjs_arg_get<GObject*>(args[0]));
         if (gobj) {
             this_object = ObjectInstance::wrapper_from_gobject(context, gobj);
@@ -301,7 +284,7 @@ static void gjs_callback_closure(ffi_cif* cif [[maybe_unused]], void* result,
 
     JS::RootedValue rval(context);
 
-    g_callable_info_load_return_type(trampoline->info, &ret_type);
+    g_callable_info_load_return_type(m_info, &ret_type);
     bool ret_type_is_void = g_type_info_get_tag (&ret_type) == GI_TYPE_TAG_VOID;
 
     for (i = 0, n_jsargs = 0; i < n_args; i++) {
@@ -309,7 +292,7 @@ static void gjs_callback_closure(ffi_cif* cif [[maybe_unused]], void* result,
         GITypeInfo type_info;
         GjsParamType param_type;
 
-        g_callable_info_load_arg(trampoline->info, i, &arg_info);
+        g_callable_info_load_arg(m_info, i, &arg_info);
         g_arg_info_load_type(&arg_info, &type_info);
 
         /* Skip void * arguments */
@@ -324,7 +307,7 @@ static void gjs_callback_closure(ffi_cif* cif [[maybe_unused]], void* result,
         if (g_arg_info_get_direction(&arg_info) == GI_DIRECTION_INOUT)
             n_outargs++;
 
-        param_type = trampoline->param_types[i];
+        param_type = m_param_types[i];
 
         switch (param_type) {
             case PARAM_SKIPPED:
@@ -335,7 +318,8 @@ static void gjs_callback_closure(ffi_cif* cif [[maybe_unused]], void* result,
                 GITypeInfo arg_type_info;
                 JS::RootedValue length(context);
 
-                g_callable_info_load_arg(trampoline->info, array_length_pos, &array_length_arg);
+                g_callable_info_load_arg(m_info, array_length_pos,
+                                         &array_length_arg);
                 g_arg_info_load_type(&array_length_arg, &arg_type_info);
                 if (!gjs_value_from_g_argument(context, &length, &arg_type_info,
                                                args[array_length_pos + c_args_offset],
@@ -375,8 +359,7 @@ static void gjs_callback_closure(ffi_cif* cif [[maybe_unused]], void* result,
         }
     }
 
-    if (!gjs_closure_invoke(trampoline->js_function, this_object, jsargs, &rval,
-                            true))
+    if (!gjs_closure_invoke(m_js_function, this_object, jsargs, &rval, true))
         goto out;
 
     if (n_outargs == 0 && ret_type_is_void) {
@@ -385,7 +368,7 @@ static void gjs_callback_closure(ffi_cif* cif [[maybe_unused]], void* result,
         GIArgument argument;
         GITransfer transfer;
 
-        transfer = g_callable_info_get_caller_owns (trampoline->info);
+        transfer = g_callable_info_get_caller_owns(m_info);
         /* non-void return value, no out args. Should
          * be a single return value. */
         if (!gjs_value_to_g_argument(context,
@@ -406,7 +389,7 @@ static void gjs_callback_closure(ffi_cif* cif [[maybe_unused]], void* result,
          * be a single return value. */
         for (i = 0; i < n_args; i++) {
             GIArgInfo arg_info;
-            g_callable_info_load_arg(trampoline->info, i, &arg_info);
+            g_callable_info_load_arg(m_info, i, &arg_info);
             if (g_arg_info_get_direction(&arg_info) == GI_DIRECTION_IN)
                 continue;
 
@@ -422,13 +405,12 @@ static void gjs_callback_closure(ffi_cif* cif [[maybe_unused]], void* result,
             goto out;
 
         if (!is_array) {
-            JSFunction* fn = gjs_closure_get_callable(trampoline->js_function);
+            JSFunction* fn = gjs_closure_get_callable(m_js_function);
             gjs_throw(context,
                       "Function %s (%s.%s) returned unexpected value, "
                       "expecting an Array",
                       gjs_debug_string(JS_GetFunctionDisplayId(fn)).c_str(),
-                      g_base_info_get_namespace(trampoline->info),
-                      g_base_info_get_name(trampoline->info));
+                      m_info.ns(), m_info.name());
             goto out;
         }
 
@@ -440,7 +422,7 @@ static void gjs_callback_closure(ffi_cif* cif [[maybe_unused]], void* result,
 
         if (!ret_type_is_void) {
             GIArgument argument;
-            GITransfer transfer = g_callable_info_get_caller_owns(trampoline->info);
+            GITransfer transfer = g_callable_info_get_caller_owns(m_info);
 
             if (!JS_GetElement(context, out_array, elem_idx, &elem))
                 goto out;
@@ -459,7 +441,7 @@ static void gjs_callback_closure(ffi_cif* cif [[maybe_unused]], void* result,
 
         for (i = 0; i < n_args; i++) {
             GIArgInfo arg_info;
-            g_callable_info_load_arg(trampoline->info, i, &arg_info);
+            g_callable_info_load_arg(m_info, i, &arg_info);
             if (g_arg_info_get_direction(&arg_info) == GI_DIRECTION_IN)
                 continue;
 
@@ -488,17 +470,16 @@ out:
                 exit(code);
 
             /* Some other uncatchable exception, e.g. out of memory */
-            JSFunction* fn = gjs_closure_get_callable(trampoline->js_function);
+            JSFunction* fn = gjs_closure_get_callable(m_js_function);
             g_error("Function %s (%s.%s) terminated with uncatchable exception",
                     gjs_debug_string(JS_GetFunctionDisplayId(fn)).c_str(),
-                    g_base_info_get_namespace(trampoline->info),
-                    g_base_info_get_name(trampoline->info));
+                    m_info.ns(), m_info.name());
         }
 
         /* Fill in the result with some hopefully neutral value */
         if (!ret_type_is_void) {
             GIArgument argument = {};
-            g_callable_info_load_return_type(trampoline->info, &ret_type);
+            g_callable_info_load_return_type(m_info, &ret_type);
             gjs_gi_argument_init_default(&ret_type, &argument);
             set_return_ffi_arg_from_giargument(&ret_type, result, &argument);
         }
@@ -523,10 +504,10 @@ out:
         gjs_log_exception_uncaught(context);
     }
 
-    if (trampoline->scope == GI_SCOPE_TYPE_ASYNC) {
+    if (m_scope == GI_SCOPE_TYPE_ASYNC) {
         // We don't release the trampoline here as we've an extra ref that has
         // been set in gjs_marshal_callback_in()
-        completed_trampolines.emplace_back(trampoline.get());
+        completed_trampolines.emplace_back(this);
     }
 
     gjs->schedule_gc_if_needed();
@@ -536,31 +517,51 @@ GjsCallbackTrampoline* gjs_callback_trampoline_new(
     JSContext* context, JS::HandleFunction function,
     GICallableInfo* callable_info, GIScopeType scope, bool has_scope_object,
     bool is_vfunc) {
-    GjsAutoCallbackTrampoline trampoline;
-    int n_args, i;
-
     g_assert(function);
 
-    trampoline = g_new(GjsCallbackTrampoline, 1);
-    new (trampoline) GjsCallbackTrampoline();
-    g_atomic_ref_count_init(&trampoline->ref_count);
-    trampoline->info = callable_info;
-    g_base_info_ref((GIBaseInfo*)trampoline->info);
+    GjsAutoCallbackTrampoline trampoline =
+        new GjsCallbackTrampoline(callable_info, scope, is_vfunc);
 
-    /* Analyze param types and directions, similarly to init_cached_function_data */
-    n_args = g_callable_info_get_n_args(trampoline->info);
-    trampoline->param_types = g_new0(GjsParamType, n_args);
+    if (!trampoline->initialize(context, function, has_scope_object))
+        return nullptr;
 
-    for (i = 0; i < n_args; i++) {
+    return trampoline.release();
+}
+
+GjsCallbackTrampoline::GjsCallbackTrampoline(GICallableInfo* callable_info,
+                                             GIScopeType scope, bool is_vfunc)
+    : m_info(g_base_info_ref(callable_info)),
+      m_scope(scope),
+      m_param_types(g_callable_info_get_n_args(callable_info), {}),
+      m_is_vfunc(is_vfunc) {
+    g_atomic_ref_count_init(&ref_count);
+}
+
+GjsCallbackTrampoline::~GjsCallbackTrampoline() {
+    g_assert(g_atomic_ref_count_compare(&ref_count, 0));
+
+    if (m_info && m_closure)
+        g_callable_info_free_closure(m_info, m_closure);
+}
+
+bool GjsCallbackTrampoline::initialize(JSContext* cx,
+                                       JS::HandleFunction function,
+                                       bool has_scope_object) {
+    g_assert(!m_js_function);
+    g_assert(!m_closure);
+
+    /* Analyze param types and directions, similarly to
+     * init_cached_function_data */
+    for (size_t i = 0; i < m_param_types.size(); i++) {
         GIDirection direction;
         GIArgInfo arg_info;
         GITypeInfo type_info;
         GITypeTag type_tag;
 
-        if (trampoline->param_types[i] == PARAM_SKIPPED)
+        if (m_param_types[i] == PARAM_SKIPPED)
             continue;
 
-        g_callable_info_load_arg(trampoline->info, i, &arg_info);
+        g_callable_info_load_arg(m_info, i, &arg_info);
         g_arg_info_load_type(&arg_info, &type_info);
 
         direction = g_arg_info_get_direction(&arg_info);
@@ -578,52 +579,62 @@ GjsCallbackTrampoline* gjs_callback_trampoline_new(
                 g_type_info_get_interface(&type_info);
             interface_type = g_base_info_get_type(interface_info);
             if (interface_type == GI_INFO_TYPE_CALLBACK) {
-                gjs_throw(context,
+                gjs_throw(cx,
                           "%s %s accepts another callback as a parameter. This "
                           "is not supported",
-                          is_vfunc ? "VFunc" : "Callback",
-                          g_base_info_get_name(callable_info));
-                return NULL;
+                          m_is_vfunc ? "VFunc" : "Callback", m_info.name());
+                return false;
             }
         } else if (type_tag == GI_TYPE_TAG_ARRAY) {
             if (g_type_info_get_array_type(&type_info) == GI_ARRAY_TYPE_C) {
                 int array_length_pos = g_type_info_get_array_length(&type_info);
 
-                if (array_length_pos >= 0 && array_length_pos < n_args) {
+                if (array_length_pos < 0)
+                    continue;
+
+                if (static_cast<size_t>(array_length_pos) <
+                    m_param_types.size()) {
                     GIArgInfo length_arg_info;
 
-                    g_callable_info_load_arg(trampoline->info, array_length_pos, &length_arg_info);
+                    g_callable_info_load_arg(m_info, array_length_pos,
+                                             &length_arg_info);
                     if (g_arg_info_get_direction(&length_arg_info) != direction) {
-                        gjs_throw(context,
+                        gjs_throw(cx,
                                   "%s %s has an array with different-direction "
                                   "length argument. This is not supported",
-                                  is_vfunc ? "VFunc" : "Callback",
-                                  g_base_info_get_name(callable_info));
-                        return NULL;
+                                  m_is_vfunc ? "VFunc" : "Callback",
+                                  m_info.name());
+                        return false;
                     }
 
-                    trampoline->param_types[array_length_pos] = PARAM_SKIPPED;
-                    trampoline->param_types[i] = PARAM_ARRAY;
+                    m_param_types[array_length_pos] = PARAM_SKIPPED;
+                    m_param_types[i] = PARAM_ARRAY;
                 }
             }
         }
     }
 
-    trampoline->closure = g_callable_info_prepare_closure(callable_info, &trampoline->cif,
-                                                          gjs_callback_closure, trampoline);
+    m_closure = g_callable_info_prepare_closure(
+        m_info, &m_cif,
+        [](ffi_cif*, void* result, void** ffi_args, void* data) {
+            auto** args = reinterpret_cast<GIArgument**>(ffi_args);
+            g_assert(data && "Trampoline data is not set");
+            GjsAutoCallbackTrampoline trampoline(
+                static_cast<GjsCallbackTrampoline*>(data),
+                GjsAutoTakeOwnership());
+
+            trampoline->callback_closure(args, result);
+        },
+        this);
 
     // The rule is:
     // - notify callbacks in GObject methods are traced from the scope object
     // - async and call callbacks, and other notify callbacks, are rooted
     // - vfuncs are traced from the GObject prototype
-    bool should_root = scope != GI_SCOPE_TYPE_NOTIFIED || !has_scope_object;
-    trampoline->js_function = gjs_closure_new(
-        context, function, g_base_info_get_name(callable_info), should_root);
-
-    trampoline->scope = scope;
-    trampoline->is_vfunc = is_vfunc;
+    bool should_root = m_scope != GI_SCOPE_TYPE_NOTIFIED || !has_scope_object;
+    m_js_function = gjs_closure_new(cx, function, m_info.name(), should_root);
 
-    return trampoline.release();
+    return true;
 }
 
 /* Intended for error messages. Return value must be freed */
diff --git a/gi/function.h b/gi/function.h
index f50b747a..75f71486 100644
--- a/gi/function.h
+++ b/gi/function.h
@@ -26,6 +26,8 @@
 
 #include <config.h>
 
+#include <vector>
+
 #include <ffi.h>
 #include <girepository.h>
 #include <glib-object.h>
@@ -49,17 +51,35 @@ typedef enum {
     PARAM_UNKNOWN,
 } GjsParamType;
 
+using GjsAutoGClosure =
+    GjsAutoPointer<GClosure, GClosure, g_closure_unref, g_closure_ref>;
+
 struct GjsCallbackTrampoline {
+    GjsCallbackTrampoline(GICallableInfo* callable_info, GIScopeType scope,
+                          bool is_vfunc);
+    ~GjsCallbackTrampoline();
+
+    constexpr GClosure* js_function() { return m_js_function; }
+    constexpr ffi_closure* closure() { return m_closure; }
+
     gatomicrefcount ref_count;
-    GICallableInfo *info;
 
-    GClosure *js_function;
+    bool initialize(JSContext* cx, JS::HandleFunction function,
+                    bool has_scope_object);
+
+ private:
+    void callback_closure(GIArgument** args, void* result);
+    void warn_about_illegal_js_callback(const char* when, const char* reason);
+
+    GjsAutoCallableInfo m_info;
+    GjsAutoGClosure m_js_function;
+
+    ffi_closure* m_closure = nullptr;
+    GIScopeType m_scope;
+    std::vector<GjsParamType> m_param_types;
 
-    ffi_cif cif;
-    ffi_closure *closure;
-    GIScopeType scope;
-    bool is_vfunc;
-    GjsParamType *param_types;
+    bool m_is_vfunc;
+    ffi_cif m_cif;
 };
 
 GJS_JSAPI_RETURN_CONVENTION
diff --git a/gi/object.cpp b/gi/object.cpp
index d0fd4915..fd3292a2 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -1461,8 +1461,6 @@ static void invalidate_closure_list(std::forward_list<GClosure*>* closures) {
         // invalidation mechanism, but adding a temporary reference to
         // ensure that the closure is still valid when calling invalidation
         // notify callbacks
-        using GjsAutoGClosure =
-            GjsAutoPointer<GClosure, GClosure, g_closure_unref, g_closure_ref>;
         GjsAutoGClosure closure(closures->front(), GjsAutoTakeOwnership());
         g_closure_invalidate(closure);
         /* Erase element if not already erased */
@@ -2624,19 +2622,19 @@ bool ObjectPrototype::hook_up_vfunc_impl(JSContext* cx,
         // This is traced, and will be cleared from the list when the closure is
         // invalidated
         g_assert(std::find(m_vfuncs.begin(), m_vfuncs.end(),
-                           trampoline->js_function) == m_vfuncs.end() &&
+                           trampoline->js_function()) == m_vfuncs.end() &&
                  "This vfunc was already associated with this class");
-        m_vfuncs.push_front(trampoline->js_function);
+        m_vfuncs.push_front(trampoline->js_function());
         g_closure_add_invalidate_notifier(
-            trampoline->js_function, this,
+            trampoline->js_function(), this,
             &ObjectPrototype::vfunc_invalidated_notify);
         g_closure_add_invalidate_notifier(
-            trampoline->js_function, trampoline, [](void* data, GClosure*) {
+            trampoline->js_function(), trampoline, [](void* data, GClosure*) {
                 auto* trampoline = static_cast<GjsCallbackTrampoline*>(data);
                 gjs_callback_trampoline_unref(trampoline);
             });
 
-        *((ffi_closure **)method_ptr) = trampoline->closure;
+        *reinterpret_cast<ffi_closure**>(method_ptr) = trampoline->closure();
     }
 
     return true;