[gjs: 1/6] closure: Reimplement to be a C++ class with custom heap allocator

[Philip Chimento <pchimento src gnome org>]

commit 189cd4b7a0f223e2259934e77bb2c1959530989a
Author: Marco Trevisan (Treviño) <mail 3v1n0 net>
Date:   Fri May 14 03:18:16 2021 +0200

    closure: Reimplement to be a C++ class with custom heap allocator
    
    To create closures we used to create an internal closure structure that
    for which we were handling construction and destruction manually.
    
    C++ allows us to use some nicer features though, and we can take
    advantage of them once we leave the role of allocating and deallocating
    the memory to GClosure functions.
    
    This can be nicely done overriding the new and delete operators that
    will give us the size to allocate exactly as the closure creator
    function expects to receive.

 gi/arg-cache.cpp |   3 +-
 gi/arg.cpp       |   3 +-
 gi/closure.cpp   | 249 +++++++++++++++++++------------------------------------
 gi/closure.h     | 102 ++++++++++++++++++++---
 gi/function.cpp  |  17 ++--
 gi/function.h    |   3 +-
 gi/object.cpp    |  11 ++-
 gi/value.cpp     |  48 +++--------
 gi/value.h       |   7 --
 9 files changed, 202 insertions(+), 241 deletions(-)
---
diff --git a/gi/arg-cache.cpp b/gi/arg-cache.cpp
index dfc76557..c93ffa99 100644
--- a/gi/arg-cache.cpp
+++ b/gi/arg-cache.cpp
@@ -29,6 +29,7 @@
 #include "gi/arg-types-inl.h"
 #include "gi/arg.h"
 #include "gi/boxed.h"
+#include "gi/closure.h"
 #include "gi/foreign.h"
 #include "gi/function.h"
 #include "gi/fundamental.h"
@@ -635,7 +636,7 @@ static bool gjs_marshal_gclosure_in_in(JSContext* cx, GjsArgumentCache* self,
                                       ExpectedType::FUNCTION);
 
     JS::RootedFunction func(cx, JS_GetObjectFunction(&value.toObject()));
-    GClosure* closure = gjs_closure_new_marshaled(cx, func, "boxed");
+    GClosure* closure = Gjs::Closure::create_marshaled(cx, func, "boxed");
     gjs_arg_set(arg, closure);
     g_closure_ref(closure);
     g_closure_sink(closure);
diff --git a/gi/arg.cpp b/gi/arg.cpp
index 2a82b07b..63e8e03c 100644
--- a/gi/arg.cpp
+++ b/gi/arg.cpp
@@ -31,6 +31,7 @@
 #include "gi/arg-types-inl.h"
 #include "gi/arg.h"
 #include "gi/boxed.h"
+#include "gi/closure.h"
 #include "gi/foreign.h"
 #include "gi/fundamental.h"
 #include "gi/gerror.h"
@@ -1342,7 +1343,7 @@ static bool value_to_interface_gi_argument(
 
             } else if (g_type_is_a(gtype, G_TYPE_BOXED)) {
                 if (g_type_is_a(gtype, G_TYPE_CLOSURE)) {
-                    GClosure* closure = gjs_closure_new_marshaled(
+                    GClosure* closure = Gjs::Closure::create_marshaled(
                         cx, JS_GetObjectFunction(obj), "boxed");
                     // GI doesn't know about floating GClosure references. We
                     // guess that if this is a return value going from JS::Value
diff --git a/gi/closure.cpp b/gi/closure.cpp
index ec34b4b9..30b64abd 100644
--- a/gi/closure.cpp
+++ b/gi/closure.cpp
@@ -1,12 +1,12 @@
 /* -*- mode: C++; c-basic-offset: 4; indent-tabs-mode: nil; -*- */
 // SPDX-License-Identifier: MIT OR LGPL-2.0-or-later
 // SPDX-FileCopyrightText: 2008 litl, LLC
+// SPDX-FileCopyrightText: 2021 Canonical Ltd.
+// SPDX-FileContributor: Marco Trevisan <marco trevisan canonical com>
 
 #include <config.h>
 
-#include <glib.h>
-
-#include <new>
+#include <glib.h>  // for g_assert
 
 #include <js/RootingAPI.h>
 #include <js/TypeDecls.h>
@@ -20,18 +20,45 @@
 #include "gjs/mem-private.h"
 #include "util/log.h"
 
-struct Closure {
-    JSContext *context;
-    GjsMaybeOwned<JSFunction*> func;
-};
+namespace Gjs {
 
-struct GjsClosure {
-    GClosure base;
+Closure::Closure(JSContext* cx, JSFunction* callable, bool root,
+                 const char* description GJS_USED_VERBOSE_GCLOSURE)
+    : m_cx(cx) {
+    GJS_INC_COUNTER(closure);
+    GClosureNotify closure_notify;
+
+    if (root) {
+        // Fully manage closure lifetime if so asked
+        auto* gjs = GjsContextPrivate::from_cx(cx);
+        g_assert(cx == gjs->context());
+        m_func.root(
+            cx, callable,
+            [](JS::HandleFunction, void* data) {
+                static_cast<Closure*>(data)->global_context_finalized();
+            },
+            this);
+        closure_notify = [](void*, GClosure* closure) {
+            static_cast<Closure*>(closure)->closure_invalidated();
+        };
+    } else {
+        // Only mark the closure as invalid if memory is managed
+        // outside (i.e. by object.c for signals)
+        m_func = callable;
+        closure_notify = [](void*, GClosure* closure) {
+            static_cast<Closure*>(closure)->closure_set_invalid();
+        };
+    }
 
-    /* We need a separate object to be able to call
-       the C++ constructor without stomping on the base */
-    Closure priv;
-};
+    g_closure_add_invalidate_notifier(this, nullptr, closure_notify);
+    g_closure_add_finalize_notifier(
+        this, nullptr, [](void*, GClosure* closure) {
+            static_cast<Closure*>(closure)->~Closure();
+        });
+
+    gjs_debug_closure("Create closure %p which calls function %p '%s'", this,
+                      m_func.debug_addr(), description);
+}
 
 /*
  * Memory management of closures is "interesting" because we're keeping around
@@ -67,40 +94,26 @@ struct GjsClosure {
  *
  */
 
-static void
-invalidate_js_pointers(GjsClosure *gc)
-{
-    Closure *c;
-
-    c = &gc->priv;
-
-    if (!c->func)
+void Closure::invalidate_js_pointers() {
+    if (!m_func)
         return;
 
-    c->func.reset();
-    c->context = nullptr;
+    reset();
 
     /* Notify any closure reference holders they
      * may want to drop references.
      */
-    g_closure_invalidate(&gc->base);
+    g_closure_invalidate(this);
 }
 
-static void global_context_finalized(JS::HandleFunction func [[maybe_unused]],
-                                     void* data) {
-    GjsClosure *gc = (GjsClosure*) data;
-    Closure *c = &gc->priv;
-
+void Closure::global_context_finalized() {
     gjs_debug_closure(
         "Context global object destroy notifier on closure %p which calls "
         "object %p",
-        c, c->func.debug_addr());
+        this, m_func.debug_addr());
 
-    if (c->func) {
-        g_assert(c->func == func.get());
-
-        invalidate_js_pointers(gc);
-    }
+    if (m_func)
+        invalidate_js_pointers();
 }
 
 /* Invalidation is like "dispose" - it is guaranteed to happen at
@@ -114,18 +127,13 @@ static void global_context_finalized(JS::HandleFunction func [[maybe_unused]],
  *
  * Unlike "dispose" invalidation only happens once.
  */
-static void closure_invalidated(void*, GClosure* closure) {
-    Closure *c;
-
-    c = &((GjsClosure*) closure)->priv;
-
+void Closure::closure_invalidated() {
     GJS_DEC_COUNTER(closure);
-    gjs_debug_closure("Invalidating closure %p which calls function %p",
-                      closure, c->func.debug_addr());
+    gjs_debug_closure("Invalidating closure %p which calls function %p", this,
+                      m_func.debug_addr());
 
-    if (!c->func) {
-        gjs_debug_closure("   (closure %p already dead, nothing to do)",
-                          closure);
+    if (!m_func) {
+        gjs_debug_closure("   (closure %p already dead, nothing to do)", this);
         return;
     }
 
@@ -136,153 +144,62 @@ static void closure_invalidated(void*, GClosure* closure) {
      * invalidated for some reason other than destruction of the
      * JSContext.
      */
-    gjs_debug_closure("   (closure %p's context was alive, "
-                      "removing our destroy notifier on global object)",
-                      closure);
+    gjs_debug_closure(
+        "   (closure %p's context was alive, "
+        "removing our destroy notifier on global object)",
+        this);
 
-    c->func.reset();
-    c->context = nullptr;
+    reset();
 }
 
-static void closure_set_invalid(void*, GClosure* closure) {
-    Closure *self = &((GjsClosure*) closure)->priv;
-
+void Closure::closure_set_invalid() {
     gjs_debug_closure("Invalidating signal closure %p which calls function %p",
-                      closure, self->func.debug_addr());
+                      this, m_func.debug_addr());
 
-    self->func.prevent_collection();
-    self->func.reset();
-    self->context = nullptr;
+    m_func.prevent_collection();
+    reset();
 
     GJS_DEC_COUNTER(closure);
 }
 
-static void closure_finalize(void*, GClosure* closure) {
-    Closure *self = &((GjsClosure*) closure)->priv;
-
-    self->~Closure();
-}
-
-bool gjs_closure_invoke(GClosure* closure, JS::HandleObject this_obj,
-                        const JS::HandleValueArray& args,
-                        JS::MutableHandleValue retval) {
-    Closure *c;
-    JSContext *context;
-
-    c = &((GjsClosure*) closure)->priv;
-
-    if (!c->func) {
+bool Closure::invoke(JS::HandleObject this_obj,
+                     const JS::HandleValueArray& args,
+                     JS::MutableHandleValue retval) {
+    if (!m_func) {
         /* We were destroyed; become a no-op */
-        c->context = nullptr;
+        reset();
         return false;
     }
 
-    context = c->context;
-    JSAutoRealm ar(context, JS_GetFunctionObject(c->func));
+    JSAutoRealm ar(m_cx, JS_GetFunctionObject(m_func));
 
-    if (gjs_log_exception(context)) {
-        gjs_debug_closure("Exception was pending before invoking callback??? "
-                          "Not expected - closure %p", closure);
+    if (gjs_log_exception(m_cx)) {
+        gjs_debug_closure(
+            "Exception was pending before invoking callback??? "
+            "Not expected - closure %p",
+            this);
     }
 
-    JS::RootedFunction func(context, c->func);
-    if (!JS::Call(context, this_obj, func, args, retval)) {
+    JS::RootedFunction func(m_cx, m_func);
+    if (!JS::Call(m_cx, this_obj, func, args, retval)) {
         /* Exception thrown... */
         gjs_debug_closure(
             "Closure invocation failed (exception should have been thrown) "
             "closure %p function %p",
-            closure, c->func.debug_addr());
+            this, m_func.debug_addr());
         return false;
     }
 
-    if (gjs_log_exception_uncaught(context)) {
-        gjs_debug_closure("Closure invocation succeeded but an exception was set"
-                          " - closure %p", closure);
+    if (gjs_log_exception_uncaught(m_cx)) {
+        gjs_debug_closure(
+            "Closure invocation succeeded but an exception was set"
+            " - closure %p",
+            m_cx);
     }
 
-    GjsContextPrivate* gjs = GjsContextPrivate::from_cx(context);
+    GjsContextPrivate* gjs = GjsContextPrivate::from_cx(m_cx);
     gjs->schedule_gc_if_needed();
     return true;
 }
 
-bool
-gjs_closure_is_valid(GClosure *closure)
-{
-    Closure *c;
-
-    c = &((GjsClosure*) closure)->priv;
-
-    return !!c->context;
-}
-
-JSContext*
-gjs_closure_get_context(GClosure *closure)
-{
-    Closure *c;
-
-    c = &((GjsClosure*) closure)->priv;
-
-    return c->context;
-}
-
-JSFunction* gjs_closure_get_callable(GClosure* closure) {
-    Closure *c;
-
-    c = &((GjsClosure*) closure)->priv;
-
-    return c->func;
-}
-
-void
-gjs_closure_trace(GClosure *closure,
-                  JSTracer *tracer)
-{
-    Closure *c;
-
-    c = &((GjsClosure*) closure)->priv;
-
-    if (!c->func)
-        return;
-
-    c->func.trace(tracer, "signal connection");
-}
-
-GClosure* gjs_closure_new(JSContext* context, JSFunction* callable,
-                          const char* description GJS_USED_VERBOSE_GCLOSURE,
-                          bool root_function) {
-    Closure *c;
-
-    auto* gc = reinterpret_cast<GjsClosure*>(
-        g_closure_new_simple(sizeof(GjsClosure), nullptr));
-    c = new (&gc->priv) Closure();
-
-    /* The saved context is used for lifetime management, so that the closure will
-     * be torn down with the context that created it. The context could be attached to
-     * the default context of the runtime using if we wanted the closure to survive
-     * the context that created it.
-     */
-    c->context = context;
-
-    GJS_INC_COUNTER(closure);
-
-    if (root_function) {
-        /* Fully manage closure lifetime if so asked */
-        c->func.root(context, callable, global_context_finalized, gc);
-
-        g_closure_add_invalidate_notifier(&gc->base, nullptr,
-                                          closure_invalidated);
-    } else {
-        c->func = callable;
-        /* Only mark the closure as invalid if memory is managed
-           outside (i.e. by object.c for signals) */
-        g_closure_add_invalidate_notifier(&gc->base, nullptr,
-                                          closure_set_invalid);
-    }
-
-    g_closure_add_finalize_notifier(&gc->base, nullptr, closure_finalize);
-
-    gjs_debug_closure("Create closure %p which calls function %p '%s'", gc,
-                      c->func.debug_addr(), description);
-
-    return &gc->base;
-}
+}  // namespace Gjs
diff --git a/gi/closure.h b/gi/closure.h
index 87299f73..e242e7cb 100644
--- a/gi/closure.h
+++ b/gi/closure.h
@@ -1,6 +1,8 @@
 /* -*- mode: C++; c-basic-offset: 4; indent-tabs-mode: nil; -*- */
 // SPDX-License-Identifier: MIT OR LGPL-2.0-or-later
 // SPDX-FileCopyrightText: 2008 litl, LLC
+// SPDX-FileCopyrightText: 2021 Canonical Ltd.
+// SPDX-FileContributor: Marco Trevisan <marco trevisan canonical com>
 
 #ifndef GI_CLOSURE_H_
 #define GI_CLOSURE_H_
@@ -8,9 +10,14 @@
 #include <config.h>
 
 #include <glib-object.h>
+#include <stddef.h>
 
+#include <js/RootingAPI.h>
 #include <js/TypeDecls.h>
 
+#include "gi/utils-inl.h"
+#include "gjs/jsapi-util-root.h"
+#include "gjs/jsapi-util.h"
 #include "gjs/macros.h"
 
 class JSTracer;
@@ -18,20 +25,91 @@ namespace JS {
 class HandleValueArray;
 }
 
-[[nodiscard]] GClosure* gjs_closure_new(JSContext* cx, JSFunction* callable,
-                                        const char* description,
-                                        bool root_function);
+namespace Gjs {
 
-GJS_JSAPI_RETURN_CONVENTION
-bool gjs_closure_invoke(GClosure* closure, JS::HandleObject this_obj,
-                        const JS::HandleValueArray& args,
-                        JS::MutableHandleValue retval);
+class Closure : public GClosure {
+    Closure(JSContext*, JSFunction*, bool root, const char* description);
 
-[[nodiscard]] JSContext* gjs_closure_get_context(GClosure* closure);
-[[nodiscard]] bool gjs_closure_is_valid(GClosure* closure);
-[[nodiscard]] JSFunction* gjs_closure_get_callable(GClosure* closure);
+    void* operator new(size_t size) {
+        return g_closure_new_simple(size, nullptr);
+    }
 
-void       gjs_closure_trace         (GClosure     *closure,
-                                      JSTracer     *tracer);
+    void operator delete(void* p) { unref(static_cast<Closure*>(p)); }
+
+    static Closure* ref(Closure* self) {
+        return static_cast<Closure*>(g_closure_ref(self));
+    }
+    static void unref(Closure* self) { g_closure_unref(self); }
+
+ public:
+    using Ptr = GjsAutoPointer<Closure, Closure, unref, ref>;
+
+    [[nodiscard]] constexpr static Closure* for_gclosure(GClosure* gclosure) {
+        // We need to do this in order to ensure this is a constant expression
+        return static_cast<Closure*>(static_cast<void*>(gclosure));
+    }
+
+    [[nodiscard]] static Closure* create(JSContext* cx, JSFunction* callable,
+                                         const char* description, bool root) {
+        return new Closure(cx, callable, root, description);
+    }
+
+    [[nodiscard]] static Closure* create_marshaled(JSContext* cx,
+                                                   JSFunction* callable,
+                                                   const char* description) {
+        auto* self = new Closure(cx, callable, true /* root */, description);
+        g_closure_set_marshal(self, marshal_cb);
+        return self;
+    }
+
+    [[nodiscard]] static Closure* create_for_signal(JSContext* cx,
+                                                    JSFunction* callable,
+                                                    const char* description,
+                                                    int signal_id) {
+        auto* self = new Closure(cx, callable, false /* root */, description);
+        g_closure_set_meta_marshal(self, gjs_int_to_pointer(signal_id),
+                                   marshal_cb);
+        return self;
+    }
+
+    constexpr JSFunction* callable() const { return m_func; }
+    [[nodiscard]] constexpr JSContext* context() const { return m_cx; }
+    [[nodiscard]] constexpr bool is_valid() const { return !!m_cx; }
+    GJS_JSAPI_RETURN_CONVENTION bool invoke(JS::HandleObject,
+                                            const JS::HandleValueArray&,
+                                            JS::MutableHandleValue);
+
+    void trace(JSTracer* tracer) {
+        if (m_func)
+            m_func.trace(tracer, "signal connection");
+    }
+
+ private:
+    void reset() {
+        m_func.reset();
+        m_cx = nullptr;
+    }
+
+    static void marshal_cb(GClosure* closure, GValue* ret, unsigned n_params,
+                           const GValue* params, void* hint, void* data) {
+        for_gclosure(closure)->marshal(ret, n_params, params, hint, data);
+    }
+
+    void closure_invalidated();
+    void closure_set_invalid();
+    void invalidate_js_pointers();
+    void global_context_finalized();
+    void marshal(GValue* ret, unsigned n_parms, const GValue* params,
+                 void* hint, void* data);
+
+    //  The saved context is used for lifetime management, so that the closure
+    //  will be torn down with the context that created it.
+    //  The context could be attached to the default context of the runtime
+    //  using if we wanted the closure to survive the context that created it.
+    JSContext* m_cx;
+    GjsMaybeOwned<JSFunction*> m_func;
+};
+
+}  // namespace Gjs
 
 #endif  // GI_CLOSURE_H_
diff --git a/gi/function.cpp b/gi/function.cpp
index aa272471..54a43c3c 100644
--- a/gi/function.cpp
+++ b/gi/function.cpp
@@ -309,10 +309,9 @@ void GjsCallbackTrampoline::warn_about_illegal_js_callback(const char* when,
  * getting the return value back.
  */
 void GjsCallbackTrampoline::callback_closure(GIArgument** args, void* result) {
-    JSContext *context;
     GITypeInfo ret_type;
 
-    if (G_UNLIKELY(!gjs_closure_is_valid(m_js_function))) {
+    if (G_UNLIKELY(!m_js_function->is_valid())) {
         warn_about_illegal_js_callback(
             "during shutdown",
             "destroying a Clutter actor or GTK widget with ::destroy signal "
@@ -321,7 +320,7 @@ void GjsCallbackTrampoline::callback_closure(GIArgument** args, void* result) {
         return;
     }
 
-    context = gjs_closure_get_context(m_js_function);
+    JSContext* context = m_js_function->context();
     GjsContextPrivate* gjs = GjsContextPrivate::from_cx(context);
     if (G_UNLIKELY(gjs->sweeping())) {
         warn_about_illegal_js_callback(
@@ -338,8 +337,7 @@ void GjsCallbackTrampoline::callback_closure(GIArgument** args, void* result) {
         return;
     }
 
-    JSAutoRealm ar(
-        context, JS_GetFunctionObject(gjs_closure_get_callable(m_js_function)));
+    JSAutoRealm ar(context, JS_GetFunctionObject(m_js_function->callable()));
 
     int n_args = m_param_types.size();
     g_assert(n_args >= 0);
@@ -400,7 +398,7 @@ void GjsCallbackTrampoline::callback_closure(GIArgument** args, void* result) {
                 exit(code);
 
             // Some other uncatchable exception, e.g. out of memory
-            JSFunction* fn = gjs_closure_get_callable(m_js_function);
+            JSFunction* fn = m_js_function->callable();
             g_error("Function %s (%s.%s) terminated with uncatchable exception",
                     gjs_debug_string(JS_GetFunctionDisplayId(fn)).c_str(),
                     m_info.ns(), m_info.name());
@@ -522,7 +520,7 @@ bool GjsCallbackTrampoline::callback_closure_inner(
         }
     }
 
-    if (!gjs_closure_invoke(m_js_function, this_object, jsargs, rval))
+    if (!m_js_function->invoke(this_object, jsargs, rval))
         return false;
 
     if (n_outargs == 0 && ret_type_is_void) {
@@ -562,7 +560,7 @@ bool GjsCallbackTrampoline::callback_closure_inner(
             return false;
 
         if (!is_array) {
-            JSFunction* fn = gjs_closure_get_callable(m_js_function);
+            JSFunction* fn = m_js_function->callable();
             gjs_throw(context,
                       "Function %s (%s.%s) returned unexpected value, "
                       "expecting an Array",
@@ -735,7 +733,8 @@ bool GjsCallbackTrampoline::initialize(JSContext* cx,
     // - async and call callbacks, and other notify callbacks, are rooted
     // - vfuncs are traced from the GObject prototype
     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);
+    m_js_function =
+        Gjs::Closure::create(cx, function, m_info.name(), should_root);
 
     return true;
 }
diff --git a/gi/function.h b/gi/function.h
index 8d348ee2..d168c315 100644
--- a/gi/function.h
+++ b/gi/function.h
@@ -20,6 +20,7 @@
 #include <js/TypeDecls.h>
 #include <js/Value.h>  // IWYU pragma: keep
 
+#include "gi/closure.h"
 #include "gjs/jsapi-util.h"
 #include "gjs/macros.h"
 
@@ -61,7 +62,7 @@ struct GjsCallbackTrampoline {
     void warn_about_illegal_js_callback(const char* when, const char* reason);
 
     GjsAutoCallableInfo m_info;
-    GjsAutoGClosure m_js_function;
+    Gjs::Closure::Ptr m_js_function;
 
     ffi_closure* m_closure = nullptr;
     GIScopeType m_scope;
diff --git a/gi/object.cpp b/gi/object.cpp
index a834a5be..036a93b3 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -1753,7 +1753,7 @@ bool ObjectInstance::constructor_impl(JSContext* context,
 
 void ObjectInstance::trace_impl(JSTracer* tracer) {
     for (GClosure *closure : m_closures)
-        gjs_closure_trace(closure, tracer);
+        Gjs::Closure::for_gclosure(closure)->trace(tracer);
 }
 
 void ObjectPrototype::trace_impl(JSTracer* tracer) {
@@ -1761,7 +1761,7 @@ void ObjectPrototype::trace_impl(JSTracer* tracer) {
     m_field_cache.trace(tracer);
     m_unresolvable_cache.trace(tracer);
     for (GClosure* closure : m_vfuncs)
-        gjs_closure_trace(closure, tracer);
+        Gjs::Closure::for_gclosure(closure)->trace(tracer);
 }
 
 void ObjectInstance::finalize_impl(JSFreeOp* fop, JSObject* obj) {
@@ -2006,7 +2006,6 @@ ObjectInstance::connect_impl(JSContext          *context,
                              const JS::CallArgs& args,
                              bool                after)
 {
-    GClosure *closure;
     gulong id;
     guint signal_id;
     GQuark signal_detail;
@@ -2042,7 +2041,7 @@ ObjectInstance::connect_impl(JSContext          *context,
         return false;
     }
 
-    closure = gjs_closure_new_for_signal(
+    GClosure* closure = Gjs::Closure::create_for_signal(
         context, JS_GetObjectFunction(callback), "signal callback", signal_id);
     if (closure == NULL)
         return false;
@@ -2253,7 +2252,7 @@ bool ObjectInstance::signal_find_impl(JSContext* cx, const JS::CallArgs& args) {
                                         nullptr, nullptr);
     } else {
         for (GClosure* candidate : m_closures) {
-            if (gjs_closure_get_callable(candidate) == func) {
+            if (Gjs::Closure::for_gclosure(candidate)->callable() == func) {
                 handler = g_signal_handler_find(m_ptr, mask, signal_id, detail,
                                                 candidate, nullptr, nullptr);
                 if (handler != 0)
@@ -2329,7 +2328,7 @@ bool ObjectInstance::signals_action_impl(JSContext* cx,
     } else {
         std::vector<GClosure*> candidates;
         for (GClosure* candidate : m_closures) {
-            if (gjs_closure_get_callable(candidate) == func)
+            if (Gjs::Closure::for_gclosure(candidate)->callable() == func)
                 candidates.push_back(candidate);
         }
         for (GClosure* candidate : candidates) {
diff --git a/gi/value.cpp b/gi/value.cpp
index df427464..ec64f0ac 100644
--- a/gi/value.cpp
+++ b/gi/value.cpp
@@ -116,29 +116,23 @@ gjs_value_from_array_and_length_values(JSContext             *context,
                                          &array_arg, array_length.toInt32());
 }
 
-static void
-closure_marshal(GClosure        *closure,
-                GValue          *return_value,
-                guint            n_param_values,
-                const GValue    *param_values,
-                gpointer         invocation_hint,
-                gpointer         marshal_data)
-{
+// FIXME(3v1n0): Move into closure.cpp one day...
+void Gjs::Closure::marshal(GValue* return_value, unsigned n_param_values,
+                           const GValue* param_values, void* invocation_hint,
+                           void* marshal_data) {
     JSContext *context;
     unsigned i;
     GSignalQuery signal_query = { 0, };
     GISignalInfo *signal_info;
 
-    gjs_debug_marshal(GJS_DEBUG_GCLOSURE,
-                      "Marshal closure %p",
-                      closure);
+    gjs_debug_marshal(GJS_DEBUG_GCLOSURE, "Marshal closure %p", this);
 
-    if (!gjs_closure_is_valid(closure)) {
+    if (!is_valid()) {
         /* We were destroyed; become a no-op */
         return;
     }
 
-    context = gjs_closure_get_context(closure);
+    context = m_cx;
     GjsContextPrivate* gjs = GjsContextPrivate::from_cx(context);
     if (G_UNLIKELY(gjs->sweeping())) {
         GSignalInvocationHint *hint = (GSignalInvocationHint*) invocation_hint;
@@ -161,7 +155,7 @@ closure_marshal(GClosure        *closure,
         return;
     }
 
-    JSFunction* func = gjs_closure_get_callable(closure);
+    JSFunction* func = callable();
     JSAutoRealm ar(context, JS_GetFunctionObject(func));
 
     if (marshal_data) {
@@ -263,7 +257,7 @@ closure_marshal(GClosure        *closure,
 
     JS::RootedValue rval(context);
 
-    if (!gjs_closure_invoke(closure, nullptr, argv, &rval)) {
+    if (!invoke(nullptr, argv, &rval)) {
         if (JS_IsExceptionPending(context)) {
             gjs_log_exception_uncaught(context);
         } else {
@@ -274,7 +268,7 @@ closure_marshal(GClosure        *closure,
                 exit(code);
 
             // Some other uncatchable exception, e.g. out of memory
-            JSFunction* fn = gjs_closure_get_callable(closure);
+            JSFunction* fn = callable();
             g_error("Function %s terminated with uncatchable exception",
                     gjs_debug_string(JS_GetFunctionDisplayId(fn)).c_str());
         }
@@ -300,28 +294,6 @@ closure_marshal(GClosure        *closure,
     }
 }
 
-GClosure* gjs_closure_new_for_signal(JSContext* context, JSFunction* callable,
-                                     const char* description, guint signal_id) {
-    GClosure *closure;
-
-    closure = gjs_closure_new(context, callable, description, false);
-
-    g_closure_set_meta_marshal(closure, GUINT_TO_POINTER(signal_id), closure_marshal);
-
-    return closure;
-}
-
-GClosure* gjs_closure_new_marshaled(JSContext* context, JSFunction* callable,
-                                    const char* description) {
-    GClosure *closure;
-
-    closure = gjs_closure_new(context, callable, description, true);
-
-    g_closure_set_marshal(closure, closure_marshal);
-
-    return closure;
-}
-
 GJS_JSAPI_RETURN_CONVENTION
 static bool gjs_value_guess_g_type(JSContext* context, JS::Value value,
                                    GType* gtype_out) {
diff --git a/gi/value.h b/gi/value.h
index 8d05ee67..99aeb09e 100644
--- a/gi/value.h
+++ b/gi/value.h
@@ -77,12 +77,5 @@ bool gjs_value_from_g_value(JSContext             *context,
                             JS::MutableHandleValue value_p,
                             const GValue          *gvalue);
 
-[[nodiscard]] GClosure* gjs_closure_new_marshaled(JSContext* cx,
-                                                  JSFunction* callable,
-                                                  const char* description);
-[[nodiscard]] GClosure* gjs_closure_new_for_signal(JSContext* cx,
-                                                   JSFunction* callable,
-                                                   const char* description,
-                                                   unsigned signal_id);
 
 #endif  // GI_VALUE_H_