[gjs: 10/12] function: Use c++ features to handle GjsCallbackTrampoline
- From: Philip Chimento <pchimento src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gjs: 10/12] function: Use c++ features to handle GjsCallbackTrampoline
- Date: Sun, 20 Sep 2020 22:13:03 +0000 (UTC)
commit cecbb4984967d3effb9bf84dd6e3c236d1b70e6f
Author: Marco Trevisan (Treviño) <mail 3v1n0 net>
Date: Thu Sep 3 23:18:12 2020 +0200
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.
gi/arg-cache.cpp | 4 +-
gi/function.cpp | 195 +++++++++++++++++++++++++++++--------------------------
gi/function.h | 34 ++++++++--
gi/object.cpp | 12 ++--
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;
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
