[gjs: 8/11] closure: Store JSFunction* pointer instead of JSObject*
- From: Cosimo Cecchi <cosimoc src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gjs: 8/11] closure: Store JSFunction* pointer instead of JSObject*
- Date: Sun, 4 Nov 2018 18:00:33 +0000 (UTC)
commit 7140cac80c8a1df90debfec2305738c830b335f7
Author: Philip Chimento <philip chimento gmail com>
Date: Sat Nov 3 22:09:05 2018 -0400
closure: Store JSFunction* pointer instead of JSObject*
In practice not much difference, but this should give a small extra
safeguard against accidentally using a non-function object as the
callable of a signal connection.
gi/arg.cpp | 10 +++----
gi/closure.cpp | 81 +++++++++++++++++++++++----------------------------
gi/closure.h | 8 ++---
gi/function.cpp | 35 +++++++++-------------
gi/function.h | 9 ++----
gi/object.cpp | 11 +++++--
gi/value.cpp | 20 ++++---------
gi/value.h | 12 ++++----
gjs/jsapi-util-root.h | 12 ++++++--
9 files changed, 91 insertions(+), 107 deletions(-)
---
diff --git a/gi/arg.cpp b/gi/arg.cpp
index 911730bc..a4e96e94 100644
--- a/gi/arg.cpp
+++ b/gi/arg.cpp
@@ -1776,11 +1776,11 @@ gjs_value_to_g_argument(JSContext *context,
}
} else if (g_type_is_a(gtype, G_TYPE_BOXED)) {
if (g_type_is_a(gtype, G_TYPE_CLOSURE)) {
- arg->v_pointer = gjs_closure_new_marshaled(context,
- &value.toObject(),
- "boxed");
- g_closure_ref((GClosure *) arg->v_pointer);
- g_closure_sink((GClosure *) arg->v_pointer);
+ GClosure* closure = gjs_closure_new_marshaled(
+ context, JS_GetObjectFunction(obj), "boxed");
+ g_closure_ref(closure);
+ g_closure_sink(closure);
+ arg->v_pointer = closure;
} else {
/* Should have been caught above as STRUCT/BOXED/UNION */
gjs_throw(context,
diff --git a/gi/closure.cpp b/gi/closure.cpp
index e5c38413..5bf082d3 100644
--- a/gi/closure.cpp
+++ b/gi/closure.cpp
@@ -34,7 +34,7 @@
struct Closure {
JSContext *context;
- GjsMaybeOwned<JSObject *> obj;
+ GjsMaybeOwned<JSFunction*> func;
};
struct GjsClosure {
@@ -86,10 +86,10 @@ invalidate_js_pointers(GjsClosure *gc)
c = &gc->priv;
- if (c->obj == nullptr)
+ if (!c->func)
return;
- c->obj.reset();
+ c->func.reset();
c->context = NULL;
/* Notify any closure reference holders they
@@ -98,19 +98,17 @@ invalidate_js_pointers(GjsClosure *gc)
g_closure_invalidate(&gc->base);
}
-static void
-global_context_finalized(JS::HandleObject obj,
- void *data)
-{
+static void global_context_finalized(JS::HandleFunction func, void* data) {
GjsClosure *gc = (GjsClosure*) data;
Closure *c = &gc->priv;
- gjs_debug_closure("Context global object destroy notifier on closure %p "
- "which calls object %p",
- c, c->obj.get());
+ gjs_debug_closure(
+ "Context global object destroy notifier on closure %p which calls "
+ "object %p",
+ c, c->func.get());
- if (c->obj) {
- g_assert(c->obj == obj.get());
+ if (c->func) {
+ g_assert(c->func == func.get());
invalidate_js_pointers(gc);
}
@@ -136,10 +134,10 @@ closure_invalidated(gpointer data,
c = &((GjsClosure*) closure)->priv;
GJS_DEC_COUNTER(closure);
- gjs_debug_closure("Invalidating closure %p which calls object %p",
- closure, c->obj.get());
+ gjs_debug_closure("Invalidating closure %p which calls function %p",
+ closure, c->func.get());
- if (c->obj == nullptr) {
+ if (!c->func) {
gjs_debug_closure(" (closure %p already dead, nothing to do)",
closure);
return;
@@ -156,7 +154,7 @@ closure_invalidated(gpointer data,
"removing our destroy notifier on global object)",
closure);
- c->obj.reset();
+ c->func.reset();
c->context = nullptr;
}
@@ -166,11 +164,11 @@ closure_set_invalid(gpointer data,
{
Closure *self = &((GjsClosure*) closure)->priv;
- gjs_debug_closure("Invalidating signal closure %p which calls object %p",
- closure, self->obj.get());
+ gjs_debug_closure("Invalidating signal closure %p which calls function %p",
+ closure, self->func.get());
- self->obj.prevent_collection();
- self->obj.reset();
+ self->func.prevent_collection();
+ self->func.reset();
self->context = nullptr;
GJS_DEC_COUNTER(closure);
@@ -197,7 +195,7 @@ gjs_closure_invoke(GClosure *closure,
c = &((GjsClosure*) closure)->priv;
- if (c->obj == nullptr) {
+ if (!c->func) {
/* We were destroyed; become a no-op */
c->context = NULL;
return false;
@@ -205,7 +203,7 @@ gjs_closure_invoke(GClosure *closure,
context = c->context;
JSAutoRequest ar(context);
- JSAutoCompartment ac(context, c->obj);
+ JSAutoCompartment ac(context, JS_GetFunctionObject(c->func));
if (JS_IsExceptionPending(context)) {
gjs_debug_closure("Exception was pending before invoking callback??? "
@@ -213,12 +211,13 @@ gjs_closure_invoke(GClosure *closure,
gjs_log_exception(context);
}
- JS::RootedValue v_closure(context, JS::ObjectValue(*c->obj));
- if (!gjs_call_function_value(context, this_obj, v_closure, args, retval)) {
+ JS::RootedFunction func(context, c->func);
+ if (!JS::Call(context, this_obj, func, args, retval)) {
/* Exception thrown... */
- gjs_debug_closure("Closure invocation failed (exception should "
- "have been thrown) closure %p callable %p",
- closure, c->obj.get());
+ gjs_debug_closure(
+ "Closure invocation failed (exception should have been thrown) "
+ "closure %p function %p",
+ closure, c->func.get());
/* If an exception has been thrown, log it, unless the caller
* explicitly wants to handle it manually (for example to turn it
* into a GError), in which case it replaces the return value
@@ -241,7 +240,7 @@ gjs_closure_invoke(GClosure *closure,
" - closure %p", closure);
}
- JS_MaybeGC(context);
+ gjs_schedule_gc_if_needed(context);
return true;
}
@@ -265,14 +264,12 @@ gjs_closure_get_context(GClosure *closure)
return c->context;
}
-JSObject*
-gjs_closure_get_callable(GClosure *closure)
-{
+JSFunction* gjs_closure_get_callable(GClosure* closure) {
Closure *c;
c = &((GjsClosure*) closure)->priv;
- return c->obj;
+ return c->func;
}
void
@@ -283,18 +280,14 @@ gjs_closure_trace(GClosure *closure,
c = &((GjsClosure*) closure)->priv;
- if (c->obj == nullptr)
+ if (!c->func)
return;
- c->obj.trace(tracer, "signal connection");
+ c->func.trace(tracer, "signal connection");
}
-GClosure*
-gjs_closure_new(JSContext *context,
- JSObject *callable,
- const char *description,
- bool root_function)
-{
+GClosure* gjs_closure_new(JSContext* context, JSFunction* callable,
+ const char* description, bool root_function) {
GjsClosure *gc;
Closure *c;
@@ -313,11 +306,11 @@ gjs_closure_new(JSContext *context,
if (root_function) {
/* Fully manage closure lifetime if so asked */
- c->obj.root(context, callable, global_context_finalized, gc);
+ c->func.root(context, callable, global_context_finalized, gc);
g_closure_add_invalidate_notifier(&gc->base, NULL, closure_invalidated);
} else {
- c->obj = callable;
+ 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, NULL, closure_set_invalid);
@@ -325,8 +318,8 @@ gjs_closure_new(JSContext *context,
g_closure_add_finalize_notifier(&gc->base, NULL, closure_finalize);
- gjs_debug_closure("Create closure %p which calls object %p '%s'",
- gc, c->obj.get(), description);
+ gjs_debug_closure("Create closure %p which calls function %p '%s'", gc,
+ c->func.get(), description);
JS_EndRequest(context);
diff --git a/gi/closure.h b/gi/closure.h
index 40f5b0d8..33cc5341 100644
--- a/gi/closure.h
+++ b/gi/closure.h
@@ -33,10 +33,8 @@
G_BEGIN_DECLS
GJS_USE
-GClosure* gjs_closure_new (JSContext *context,
- JSObject *callable,
- const char *description,
- bool root_function);
+GClosure* gjs_closure_new(JSContext* cx, JSFunction* callable,
+ const char* description, bool root_function);
GJS_USE
bool gjs_closure_invoke(GClosure *closure,
@@ -50,7 +48,7 @@ JSContext* gjs_closure_get_context (GClosure *closure);
GJS_USE
bool gjs_closure_is_valid (GClosure *closure);
GJS_USE
-JSObject* gjs_closure_get_callable (GClosure *closure);
+JSFunction* gjs_closure_get_callable(GClosure* closure);
void gjs_closure_trace (GClosure *closure,
JSTracer *tracer);
diff --git a/gi/function.cpp b/gi/function.cpp
index 2673eaf5..41586f14 100644
--- a/gi/function.cpp
+++ b/gi/function.cpp
@@ -226,8 +226,8 @@ gjs_callback_closure(ffi_cif *cif,
}
JS_BeginRequest(context);
- JSAutoCompartment ac(context,
- gjs_closure_get_callable(trampoline->js_function));
+ JSAutoCompartment ac(context, JS_GetFunctionObject(gjs_closure_get_callable(
+ trampoline->js_function)));
bool can_throw_gerror = g_callable_info_can_throw_gerror(trampoline->info);
n_args = g_callable_info_get_n_args(trampoline->info);
@@ -490,19 +490,14 @@ gjs_destroy_notify_callback(gpointer data)
gjs_callback_trampoline_unref(trampoline);
}
-GjsCallbackTrampoline*
-gjs_callback_trampoline_new(JSContext *context,
- JS::HandleValue function,
- GICallableInfo *callable_info,
- GIScopeType scope,
- JS::HandleObject scope_object,
- bool is_vfunc)
-{
+GjsCallbackTrampoline* gjs_callback_trampoline_new(
+ JSContext* context, JS::HandleFunction function,
+ GICallableInfo* callable_info, GIScopeType scope,
+ JS::HandleObject scope_object, bool is_vfunc) {
GjsCallbackTrampoline *trampoline;
int n_args, i;
- g_assert(function.isObject());
- g_assert(JS_TypeOfValue(context, function) == JSTYPE_FUNCTION);
+ g_assert(function);
trampoline = g_slice_new(GjsCallbackTrampoline);
new (trampoline) GjsCallbackTrampoline();
@@ -516,9 +511,8 @@ gjs_callback_trampoline_new(JSContext *context,
* (and same for vfuncs, which are associated with a GObject prototype)
*/
bool should_root = scope != GI_SCOPE_TYPE_NOTIFIED || !scope_object;
- trampoline->js_function = gjs_closure_new(context, &function.toObject(),
- g_base_info_get_name(callable_info),
- should_root);
+ trampoline->js_function = gjs_closure_new(
+ context, function, g_base_info_get_name(callable_info), should_root);
if (!should_root && scope_object)
gjs_object_associate_closure(context, scope_object,
trampoline->js_function);
@@ -959,13 +953,12 @@ gjs_invoke_c_function(JSContext *context,
break;
}
+ JS::RootedFunction func(
+ context, JS_GetObjectFunction(¤t_arg.toObject()));
callable_info = (GICallableInfo*) g_type_info_get_interface(&ainfo);
- trampoline = gjs_callback_trampoline_new(context,
- current_arg,
- callable_info,
- scope,
- is_object_method ? obj : nullptr,
- false);
+ trampoline = gjs_callback_trampoline_new(
+ context, func, callable_info, scope,
+ is_object_method ? obj : nullptr, false);
closure = trampoline->closure;
g_base_info_unref(callable_info);
}
diff --git a/gi/function.h b/gi/function.h
index 30f473c7..42861d39 100644
--- a/gi/function.h
+++ b/gi/function.h
@@ -56,12 +56,9 @@ struct GjsCallbackTrampoline {
};
GJS_JSAPI_RETURN_CONVENTION
-GjsCallbackTrampoline* gjs_callback_trampoline_new(JSContext *context,
- JS::HandleValue function,
- GICallableInfo *callable_info,
- GIScopeType scope,
- JS::HandleObject scope_object,
- bool is_vfunc);
+GjsCallbackTrampoline* gjs_callback_trampoline_new(
+ JSContext* cx, JS::HandleFunction function, GICallableInfo* callable_info,
+ GIScopeType scope, JS::HandleObject scope_object, bool is_vfunc);
void gjs_callback_trampoline_unref(GjsCallbackTrampoline *trampoline);
void gjs_callback_trampoline_ref(GjsCallbackTrampoline *trampoline);
diff --git a/gi/object.cpp b/gi/object.cpp
index 0edb003e..ee31e1b6 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -1917,7 +1917,8 @@ ObjectInstance::connect_impl(JSContext *context,
return false;
}
- closure = gjs_closure_new_for_signal(context, callback, "signal callback", signal_id);
+ closure = gjs_closure_new_for_signal(
+ context, JS_GetObjectFunction(callback), "signal callback", signal_id);
if (closure == NULL)
return false;
associate_closure(context, closure);
@@ -2533,9 +2534,13 @@ bool ObjectPrototype::hook_up_vfunc_impl(JSContext* cx,
offset = g_field_info_get_offset(field_info);
method_ptr = G_STRUCT_MEMBER_P(implementor_vtable, offset);
- JS::RootedValue v_function(cx, JS::ObjectValue(*function));
+ if (!JS_ObjectIsFunction(cx, function)) {
+ gjs_throw(cx, "Tried to deal with a vfunc that wasn't a function");
+ return false;
+ }
+ JS::RootedFunction func(cx, JS_GetObjectFunction(function));
trampoline = gjs_callback_trampoline_new(
- cx, v_function, vfunc, GI_SCOPE_TYPE_NOTIFIED, prototype, true);
+ cx, func, vfunc, GI_SCOPE_TYPE_NOTIFIED, prototype, true);
*((ffi_closure **)method_ptr) = trampoline->closure;
}
diff --git a/gi/value.cpp b/gi/value.cpp
index fabecca2..1e51fd96 100644
--- a/gi/value.cpp
+++ b/gi/value.cpp
@@ -124,7 +124,6 @@ closure_marshal(GClosure *closure,
gpointer marshal_data)
{
JSContext *context;
- JSObject *obj;
unsigned i;
GSignalQuery signal_query = { 0, };
GISignalInfo *signal_info;
@@ -163,9 +162,9 @@ closure_marshal(GClosure *closure,
return;
}
- obj = gjs_closure_get_callable(closure);
+ JSFunction* func = gjs_closure_get_callable(closure);
JSAutoRequest ar(context);
- JSAutoCompartment ac(context, obj);
+ JSAutoCompartment ac(context, JS_GetFunctionObject(func));
if (marshal_data) {
/* we are used for a signal handler */
@@ -292,12 +291,8 @@ closure_marshal(GClosure *closure,
}
}
-GClosure*
-gjs_closure_new_for_signal(JSContext *context,
- JSObject *callable,
- const char *description,
- guint signal_id)
-{
+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);
@@ -307,11 +302,8 @@ gjs_closure_new_for_signal(JSContext *context,
return closure;
}
-GClosure*
-gjs_closure_new_marshaled (JSContext *context,
- JSObject *callable,
- const char *description)
-{
+GClosure* gjs_closure_new_marshaled(JSContext* context, JSFunction* callable,
+ const char* description) {
GClosure *closure;
closure = gjs_closure_new(context, callable, description, true);
diff --git a/gi/value.h b/gi/value.h
index af61b8d8..ab9f8194 100644
--- a/gi/value.h
+++ b/gi/value.h
@@ -47,14 +47,12 @@ bool gjs_value_from_g_value(JSContext *context,
const GValue *gvalue);
GJS_USE
-GClosure* gjs_closure_new_marshaled (JSContext *context,
- JSObject *callable,
- const char *description);
+GClosure* gjs_closure_new_marshaled(JSContext* cx, JSFunction* callable,
+ const char* description);
GJS_USE
-GClosure* gjs_closure_new_for_signal (JSContext *context,
- JSObject *callable,
- const char *description,
- guint signal_id);
+GClosure* gjs_closure_new_for_signal(JSContext* cx, JSFunction* callable,
+ const char* description,
+ unsigned signal_id);
G_END_DECLS
diff --git a/gjs/jsapi-util-root.h b/gjs/jsapi-util-root.h
index 299562bd..c81c4e55 100644
--- a/gjs/jsapi-util-root.h
+++ b/gjs/jsapi-util-root.h
@@ -87,8 +87,16 @@ struct GjsHeapOperation<JSObject *> {
}
};
-template<>
-struct GjsHeapOperation<JS::Value> {};
+template <>
+struct GjsHeapOperation<JSFunction*> {
+ static void expose_to_js(const JS::Heap<JSFunction*>& thing) {
+ JSFunction* func = thing.unbarrieredGet();
+ if (!func || !js::gc::detail::GetGCThingZone(uintptr_t(func)))
+ return;
+ if (!JS::CurrentThreadIsHeapCollecting())
+ js::gc::ExposeGCThingToActiveJS(JS::GCCellPtr(func));
+ }
+};
/* GjsMaybeOwned is intended only for use in heap allocation. Do not allocate it
* on the stack, and do not allocate any instances of structures that have it as
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
