[gjs: 2/6] GjsCallBackTrampoline: Inherit from Gjs::Closure (and so GClosure)
- From: Philip Chimento <pchimento src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gjs: 2/6] GjsCallBackTrampoline: Inherit from Gjs::Closure (and so GClosure)
- Date: Sun, 1 Aug 2021 23:01:48 +0000 (UTC)
commit 53a6cca32243225e180a544db258b89853d624cf
Author: Marco Trevisan (Treviño) <mail 3v1n0 net>
Date: Sat May 15 01:55:51 2021 +0200
GjsCallBackTrampoline: Inherit from Gjs::Closure (and so GClosure)
The callback trampoline is in fact a GClosure, but we were just composing
one. We can instead just inherit from it now, given that the
Gjs::Closure will allocate the needed memory via the closure allocator
and handle the references via native closure reference system (avoid to
duplicate that).
As per this, some less memory used as we save a pointer and the
reference counting.
The only bit we need to handle is the destruction as we need to be sure
to call the proper type destructor on closure finalization.
So, adding an utility function that ensures this happens for each type
and that we won't call the base destructor twice.
gi/arg-cache.cpp | 18 ++++++------
gi/closure.cpp | 4 ---
gi/closure.h | 17 ++++++++++-
gi/function.cpp | 86 +++++++++++++++++++++++---------------------------------
gi/function.h | 36 ++++++++----------------
gi/object.cpp | 17 +++++------
6 files changed, 78 insertions(+), 100 deletions(-)
---
diff --git a/gi/arg-cache.cpp b/gi/arg-cache.cpp
index c93ffa99..dd46b88a 100644
--- a/gi/arg-cache.cpp
+++ b/gi/arg-cache.cpp
@@ -288,9 +288,9 @@ static bool gjs_marshal_callback_in(JSContext* cx, GjsArgumentCache* self,
GjsAutoCallableInfo callable_info =
g_type_info_get_interface(&self->type_info);
bool is_object_method = !!state->instance_object;
- trampoline = gjs_callback_trampoline_new(cx, func, callable_info,
- self->contents.callback.scope,
- is_object_method, false);
+ trampoline = GjsCallbackTrampoline::create(
+ cx, func, callable_info, self->contents.callback.scope,
+ is_object_method, false);
if (!trampoline)
return false;
if (self->contents.callback.scope == GI_SCOPE_TYPE_NOTIFIED &&
@@ -301,7 +301,7 @@ static bool gjs_marshal_callback_in(JSContext* cx, GjsArgumentCache* self,
return false;
}
- if (!priv->associate_closure(cx, trampoline->js_function()))
+ if (!priv->associate_closure(cx, trampoline))
return false;
}
closure = trampoline->closure();
@@ -312,11 +312,10 @@ static bool gjs_marshal_callback_in(JSContext* cx, GjsArgumentCache* self,
GDestroyNotify destroy_notify = nullptr;
if (trampoline) {
/* Adding another reference and a DestroyNotify that unsets it */
- gjs_callback_trampoline_ref(trampoline);
+ g_closure_ref(trampoline);
destroy_notify = [](void* data) {
g_assert(data);
- gjs_callback_trampoline_unref(
- static_cast<GjsCallbackTrampoline*>(data));
+ g_closure_unref(static_cast<GClosure*>(data));
};
}
gjs_arg_set(&state->in_cvalue(destroy_pos), destroy_notify);
@@ -329,7 +328,7 @@ static bool gjs_marshal_callback_in(JSContext* cx, GjsArgumentCache* self,
if (trampoline && self->contents.callback.scope == GI_SCOPE_TYPE_ASYNC) {
// Add an extra reference that will be cleared when garbage collecting
// async calls
- gjs_callback_trampoline_ref(trampoline);
+ g_closure_ref(trampoline);
}
gjs_arg_set(arg, closure);
@@ -934,8 +933,7 @@ static bool gjs_marshal_callback_release(JSContext*, GjsArgumentCache*,
if (!closure)
return true;
- GjsAutoCallbackTrampoline trampoline =
- static_cast<GjsCallbackTrampoline*>(closure->user_data);
+ g_closure_unref(static_cast<GClosure*>(closure->user_data));
// CallbackTrampolines are refcounted because for notified/async closures
// it is possible to destroy it while in call, and therefore we cannot
// check its scope at this point
diff --git a/gi/closure.cpp b/gi/closure.cpp
index 30b64abd..3f15d078 100644
--- a/gi/closure.cpp
+++ b/gi/closure.cpp
@@ -51,10 +51,6 @@ Closure::Closure(JSContext* cx, JSFunction* callable, bool root,
}
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);
diff --git a/gi/closure.h b/gi/closure.h
index e242e7cb..cfc546ee 100644
--- a/gi/closure.h
+++ b/gi/closure.h
@@ -28,7 +28,18 @@ class HandleValueArray;
namespace Gjs {
class Closure : public GClosure {
+ protected:
Closure(JSContext*, JSFunction*, bool root, const char* description);
+ ~Closure() = default;
+
+ // Need to call this if inheriting from Closure to call the dtor
+ template <class C>
+ constexpr void add_finalize_notifier() {
+ static_assert(std::is_base_of_v<Closure, C>);
+ g_closure_add_finalize_notifier(
+ this, nullptr,
+ [](void*, GClosure* closure) { static_cast<C*>(closure)->~C(); });
+ }
void* operator new(size_t size) {
return g_closure_new_simple(size, nullptr);
@@ -51,13 +62,16 @@ class Closure : public GClosure {
[[nodiscard]] static Closure* create(JSContext* cx, JSFunction* callable,
const char* description, bool root) {
- return new Closure(cx, callable, root, description);
+ auto* self = new Closure(cx, callable, root, description);
+ self->add_finalize_notifier<Closure>();
+ return self;
}
[[nodiscard]] static Closure* create_marshaled(JSContext* cx,
JSFunction* callable,
const char* description) {
auto* self = new Closure(cx, callable, true /* root */, description);
+ self->add_finalize_notifier<Closure>();
g_closure_set_marshal(self, marshal_cb);
return self;
}
@@ -67,6 +81,7 @@ class Closure : public GClosure {
const char* description,
int signal_id) {
auto* self = new Closure(cx, callable, false /* root */, description);
+ self->add_finalize_notifier<Closure>();
g_closure_set_meta_marshal(self, gjs_int_to_pointer(signal_id),
marshal_cb);
return self;
diff --git a/gi/function.cpp b/gi/function.cpp
index 54a43c3c..12b614f4 100644
--- a/gi/function.cpp
+++ b/gi/function.cpp
@@ -178,20 +178,7 @@ class Function : public CWrapper<Function> {
* while it's in use, this list keeps track of ones that
* will be freed the next time we invoke a C function.
*/
-static std::vector<GjsAutoCallbackTrampoline> completed_trampolines;
-
-GjsCallbackTrampoline* gjs_callback_trampoline_ref(
- GjsCallbackTrampoline* trampoline) {
- g_atomic_ref_count_inc(&trampoline->ref_count);
- return trampoline;
-}
-
-void
-gjs_callback_trampoline_unref(GjsCallbackTrampoline *trampoline)
-{
- if (g_atomic_ref_count_dec(&trampoline->ref_count))
- delete trampoline;
-}
+static std::vector<Gjs::Closure::Ptr> completed_trampolines;
template <typename T, GITypeTag TAG = GI_TYPE_TAG_VOID>
static inline std::enable_if_t<std::is_integral_v<T> && std::is_signed_v<T>>
@@ -311,7 +298,7 @@ void GjsCallbackTrampoline::warn_about_illegal_js_callback(const char* when,
void GjsCallbackTrampoline::callback_closure(GIArgument** args, void* result) {
GITypeInfo ret_type;
- if (G_UNLIKELY(!m_js_function->is_valid())) {
+ if (G_UNLIKELY(!is_valid())) {
warn_about_illegal_js_callback(
"during shutdown",
"destroying a Clutter actor or GTK widget with ::destroy signal "
@@ -320,7 +307,7 @@ void GjsCallbackTrampoline::callback_closure(GIArgument** args, void* result) {
return;
}
- JSContext* context = m_js_function->context();
+ JSContext* context = this->context();
GjsContextPrivate* gjs = GjsContextPrivate::from_cx(context);
if (G_UNLIKELY(gjs->sweeping())) {
warn_about_illegal_js_callback(
@@ -337,7 +324,7 @@ void GjsCallbackTrampoline::callback_closure(GIArgument** args, void* result) {
return;
}
- JSAutoRealm ar(context, JS_GetFunctionObject(m_js_function->callable()));
+ JSAutoRealm ar(context, JS_GetFunctionObject(callable()));
int n_args = m_param_types.size();
g_assert(n_args >= 0);
@@ -398,7 +385,7 @@ void GjsCallbackTrampoline::callback_closure(GIArgument** args, void* result) {
exit(code);
// Some other uncatchable exception, e.g. out of memory
- JSFunction* fn = m_js_function->callable();
+ JSFunction* fn = 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());
@@ -520,7 +507,7 @@ bool GjsCallbackTrampoline::callback_closure_inner(
}
}
- if (!m_js_function->invoke(this_object, jsargs, rval))
+ if (!invoke(this_object, jsargs, rval))
return false;
if (n_outargs == 0 && ret_type_is_void) {
@@ -560,7 +547,7 @@ bool GjsCallbackTrampoline::callback_closure_inner(
return false;
if (!is_array) {
- JSFunction* fn = m_js_function->callable();
+ JSFunction* fn = callable();
gjs_throw(context,
"Function %s (%s.%s) returned unexpected value, "
"expecting an Array",
@@ -614,41 +601,45 @@ bool GjsCallbackTrampoline::callback_closure_inner(
return true;
}
-GjsCallbackTrampoline* gjs_callback_trampoline_new(
- JSContext* context, JS::HandleFunction function,
- GICallableInfo* callable_info, GIScopeType scope, bool has_scope_object,
- bool is_vfunc) {
+GjsCallbackTrampoline* GjsCallbackTrampoline::create(
+ JSContext* cx, JS::HandleFunction function, GICallableInfo* callable_info,
+ GIScopeType scope, bool has_scope_object, bool is_vfunc) {
g_assert(function);
+ auto* trampoline = new GjsCallbackTrampoline(
+ cx, function, callable_info, scope, has_scope_object, is_vfunc);
- GjsAutoCallbackTrampoline trampoline =
- new GjsCallbackTrampoline(callable_info, scope, is_vfunc);
-
- if (!trampoline->initialize(context, function, has_scope_object))
+ if (!trampoline->initialize()) {
+ g_closure_unref(trampoline);
return nullptr;
+ }
- return trampoline.release();
+ return trampoline;
}
-GjsCallbackTrampoline::GjsCallbackTrampoline(GICallableInfo* callable_info,
- GIScopeType scope, bool is_vfunc)
- : m_info(callable_info, GjsAutoTakeOwnership()),
+GjsCallbackTrampoline::GjsCallbackTrampoline(
+ JSContext* cx, JS::HandleFunction function, GICallableInfo* callable_info,
+ GIScopeType scope, bool has_scope_object, bool is_vfunc)
+ // The rooting 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
+ : Closure(cx, function,
+ scope != GI_SCOPE_TYPE_NOTIFIED || !has_scope_object,
+ g_base_info_get_name(callable_info)),
+ m_info(callable_info, GjsAutoTakeOwnership()),
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);
+ add_finalize_notifier<GjsCallbackTrampoline>();
}
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);
+bool GjsCallbackTrampoline::initialize() {
+ g_assert(is_valid());
g_assert(!m_closure);
/* Analyze param types and directions, similarly to
@@ -680,7 +671,7 @@ bool GjsCallbackTrampoline::initialize(JSContext* cx,
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(cx,
+ gjs_throw(context(),
"%s %s accepts another callback as a parameter. This "
"is not supported",
m_is_vfunc ? "VFunc" : "Callback", m_info.name());
@@ -700,7 +691,7 @@ bool GjsCallbackTrampoline::initialize(JSContext* cx,
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(cx,
+ gjs_throw(context(),
"%s %s has an array with different-direction "
"length argument. This is not supported",
m_is_vfunc ? "VFunc" : "Callback",
@@ -720,22 +711,15 @@ bool GjsCallbackTrampoline::initialize(JSContext* cx,
[](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(
+ Gjs::Closure::Ptr trampoline(
static_cast<GjsCallbackTrampoline*>(data),
GjsAutoTakeOwnership());
- trampoline->callback_closure(args, result);
+ trampoline.as<GjsCallbackTrampoline>()->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 = m_scope != GI_SCOPE_TYPE_NOTIFIED || !has_scope_object;
- 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 d168c315..2aae8230 100644
--- a/gi/function.h
+++ b/gi/function.h
@@ -39,20 +39,22 @@ typedef enum {
using GjsAutoGClosure =
GjsAutoPointer<GClosure, GClosure, g_closure_unref, g_closure_ref>;
-struct GjsCallbackTrampoline {
- GjsCallbackTrampoline(GICallableInfo* callable_info, GIScopeType scope,
- bool is_vfunc);
+struct GjsCallbackTrampoline : public Gjs::Closure {
+ GJS_JSAPI_RETURN_CONVENTION static GjsCallbackTrampoline* create(
+ JSContext* cx, JS::HandleFunction function,
+ GICallableInfo* callable_info, GIScopeType scope, bool has_scope_object,
+ bool is_vfunc);
+
~GjsCallbackTrampoline();
- constexpr GClosure* js_function() const { return m_js_function; }
constexpr ffi_closure* closure() const { return m_closure; }
- gatomicrefcount ref_count;
-
- bool initialize(JSContext* cx, JS::HandleFunction function,
- bool has_scope_object);
-
private:
+ GJS_JSAPI_RETURN_CONVENTION bool initialize();
+ GjsCallbackTrampoline(JSContext* cx, JS::HandleFunction function,
+ GICallableInfo* callable_info, GIScopeType scope,
+ bool has_scope_object, bool is_vfunc);
+
void callback_closure(GIArgument** args, void* result);
GJS_JSAPI_RETURN_CONVENTION
bool callback_closure_inner(JSContext* cx, JS::HandleObject this_object,
@@ -62,29 +64,15 @@ struct GjsCallbackTrampoline {
void warn_about_illegal_js_callback(const char* when, const char* reason);
GjsAutoCallableInfo m_info;
- Gjs::Closure::Ptr m_js_function;
+ GIScopeType m_scope;
ffi_closure* m_closure = nullptr;
- GIScopeType m_scope;
std::vector<GjsParamType> m_param_types;
bool m_is_vfunc;
ffi_cif m_cif;
};
-GJS_JSAPI_RETURN_CONVENTION
-GjsCallbackTrampoline* gjs_callback_trampoline_new(
- JSContext* cx, JS::HandleFunction function, GICallableInfo* callable_info,
- GIScopeType scope, bool has_scope_object, bool is_vfunc);
-
-void gjs_callback_trampoline_unref(GjsCallbackTrampoline *trampoline);
-GjsCallbackTrampoline* gjs_callback_trampoline_ref(
- GjsCallbackTrampoline* trampoline);
-
-using GjsAutoCallbackTrampoline =
- GjsAutoPointer<GjsCallbackTrampoline, GjsCallbackTrampoline,
- gjs_callback_trampoline_unref, gjs_callback_trampoline_ref>;
-
// Stack allocation only!
class GjsFunctionCallState {
GIArgument* m_in_cvalues;
diff --git a/gi/object.cpp b/gi/object.cpp
index 036a93b3..2d5519a6 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -2789,25 +2789,22 @@ bool ObjectPrototype::hook_up_vfunc_impl(JSContext* cx,
return false;
}
JS::RootedFunction func(cx, JS_GetObjectFunction(function));
- trampoline = gjs_callback_trampoline_new(
+ trampoline = GjsCallbackTrampoline::create(
cx, func, vfunc, GI_SCOPE_TYPE_NOTIFIED, true, true);
if (!trampoline)
return false;
// 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() &&
+ g_assert(std::find(m_vfuncs.begin(), m_vfuncs.end(), trampoline) ==
+ m_vfuncs.end() &&
"This vfunc was already associated with this class");
- m_vfuncs.push_front(trampoline->js_function());
+ m_vfuncs.push_front(trampoline);
g_closure_add_invalidate_notifier(
- trampoline->js_function(), this,
- &ObjectPrototype::vfunc_invalidated_notify);
+ trampoline, this, &ObjectPrototype::vfunc_invalidated_notify);
g_closure_add_invalidate_notifier(
- trampoline->js_function(), trampoline, [](void* data, GClosure*) {
- auto* trampoline = static_cast<GjsCallbackTrampoline*>(data);
- gjs_callback_trampoline_unref(trampoline);
- });
+ trampoline, nullptr,
+ [](void*, GClosure* closure) { g_closure_unref(closure); });
*reinterpret_cast<ffi_closure**>(method_ptr) = trampoline->closure();
}
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
