[gjs/wip/gdbus-2: 3/13] Associate callbacks with the object on which their installed
- From: Giovanni Campagna <gcampagna src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gjs/wip/gdbus-2: 3/13] Associate callbacks with the object on which their installed
- Date: Fri, 2 Nov 2012 13:10:14 +0000 (UTC)
commit 74f022f445a90cde4c3a5c11f2dc97ff06266df6
Author: Giovanni Campagna <gcampagna src gnome org>
Date: Sat Jul 7 03:47:25 2012 +0200
Associate callbacks with the object on which their installed
Callbacks installed with GObject methods are now traced from that
object instead of being rooted, so cycles no longer cause leaks.
This implementing by making GjsCallbackTrampoline use GjsClosure
internally, and generalizing the existing code that dealt with
signal connections.
Also, to keep tests ok, we now have class finalization functions
for custom types (to unregister the closures).
https://bugzilla.gnome.org/show_bug.cgi?id=679688
gi/closure.c | 23 ++++++++-
gi/closure.h | 4 +-
gi/function.c | 53 +++++++++++++--------
gi/function.h | 4 +-
gi/object.c | 131 +++++++++++++++++++++++++++++++++++++++-------------
gi/object.h | 5 ++-
gi/value.c | 2 +-
modules/dbus.c | 11 ++--
modules/mainloop.c | 5 +-
9 files changed, 169 insertions(+), 69 deletions(-)
---
diff --git a/gi/closure.c b/gi/closure.c
index 96a7477..57e5c7a 100644
--- a/gi/closure.c
+++ b/gi/closure.c
@@ -247,14 +247,16 @@ closure_finalized(gpointer data,
GJS_DEC_COUNTER(closure);
}
-void
+JSBool
gjs_closure_invoke(GClosure *closure,
+ JSObject *this_obj,
int argc,
jsval *argv,
jsval *retval)
{
Closure *c;
JSContext *context;
+ JSBool ret;
c = (Closure*) closure;
@@ -263,7 +265,7 @@ gjs_closure_invoke(GClosure *closure,
if (c->obj == NULL) {
/* We were destroyed; become a no-op */
c->context = NULL;
- return;
+ return JS_FALSE;
}
context = gjs_runtime_get_current_context(c->runtime);
@@ -275,8 +277,10 @@ gjs_closure_invoke(GClosure *closure,
gjs_log_exception(context, NULL);
}
+ ret = JS_FALSE;
+
if (!gjs_call_function_value(context,
- NULL, /* "this" object; NULL is some kind of default presumably */
+ this_obj,
OBJECT_TO_JSVAL(c->obj),
argc,
argv,
@@ -294,8 +298,11 @@ gjs_closure_invoke(GClosure *closure,
gjs_debug_closure("Closure invocation succeeded but an exception was set");
}
+ ret = JS_TRUE;
+
out:
JS_EndRequest(context);
+ return ret;
}
gboolean
@@ -318,6 +325,16 @@ gjs_closure_get_runtime(GClosure *closure)
return c->runtime;
}
+JSContext*
+gjs_closure_get_context(GClosure *closure)
+{
+ Closure *c;
+
+ c = (Closure*) closure;
+
+ return c->context;
+}
+
JSObject*
gjs_closure_get_callable(GClosure *closure)
{
diff --git a/gi/closure.h b/gi/closure.h
index f894805..4718015 100644
--- a/gi/closure.h
+++ b/gi/closure.h
@@ -34,11 +34,13 @@ GClosure* gjs_closure_new (JSContext *context,
JSObject *callable,
const char *description,
gboolean root_function);
-void gjs_closure_invoke (GClosure *closure,
+JSBool gjs_closure_invoke (GClosure *closure,
+ JSObject *this_obj,
int argc,
jsval *argv,
jsval *retval);
JSRuntime* gjs_closure_get_runtime (GClosure *closure);
+JSContext* gjs_closure_get_context (GClosure *closure);
gboolean gjs_closure_is_valid (GClosure *closure);
JSObject* gjs_closure_get_callable (GClosure *closure);
diff --git a/gi/function.c b/gi/function.c
index 70e50e2..efcb2e5 100644
--- a/gi/function.c
+++ b/gi/function.c
@@ -29,6 +29,7 @@
#include "boxed.h"
#include "union.h"
#include "gerror.h"
+#include "closure.h"
#include <gjs/gjs-module.h>
#include <gjs/compat.h>
@@ -80,12 +81,7 @@ gjs_callback_trampoline_unref(GjsCallbackTrampoline *trampoline)
trampoline->ref_count--;
if (trampoline->ref_count == 0) {
- JSContext *context;
-
- context = gjs_runtime_get_current_context(trampoline->runtime);
-
- if (!trampoline->is_vfunc)
- JS_RemoveValueRoot(context, &trampoline->js_function);
+ g_closure_unref(trampoline->js_function);
g_callable_info_free_closure(trampoline->info, trampoline->closure);
g_base_info_unref( (GIBaseInfo*) trampoline->info);
g_free (trampoline->param_types);
@@ -178,7 +174,7 @@ gjs_callback_closure(ffi_cif *cif,
g_assert(trampoline);
gjs_callback_trampoline_ref(trampoline);
- context = gjs_runtime_get_current_context(trampoline->runtime);
+ context = gjs_closure_get_context(trampoline->js_function);
JS_BeginRequest(context);
n_args = g_callable_info_get_n_args(trampoline->info);
@@ -251,12 +247,11 @@ gjs_callback_closure(ffi_cif *cif,
this_object = NULL;
}
- if (!JS_CallFunctionValue(context,
- this_object,
- trampoline->js_function,
- n_jsargs,
- jsargs,
- &rval)) {
+ if (!gjs_closure_invoke(trampoline->js_function,
+ this_object,
+ n_jsargs,
+ jsargs,
+ &rval)) {
goto out;
}
@@ -362,8 +357,6 @@ gjs_callback_closure(ffi_cif *cif,
out:
if (!success) {
- gjs_log_exception (context, NULL);
-
/* Fill in the result with some hopefully neutral value */
g_callable_info_load_return_type(trampoline->info, &ret_type);
gjs_g_argument_init_default (context, &ret_type, result);
@@ -394,10 +387,12 @@ gjs_callback_trampoline_new(JSContext *context,
jsval function,
GICallableInfo *callable_info,
GIScopeType scope,
+ JSObject *scope_object,
gboolean is_vfunc)
{
GjsCallbackTrampoline *trampoline;
int n_args, i;
+ gboolean should_root;
if (JSVAL_IS_NULL(function)) {
return NULL;
@@ -407,12 +402,21 @@ gjs_callback_trampoline_new(JSContext *context,
trampoline = g_slice_new(GjsCallbackTrampoline);
trampoline->ref_count = 1;
- trampoline->runtime = JS_GetRuntime(context);
trampoline->info = callable_info;
g_base_info_ref((GIBaseInfo*)trampoline->info);
- trampoline->js_function = function;
- if (!is_vfunc)
- JS_AddValueRoot(context, &trampoline->js_function);
+
+ /* The rule is:
+ * - async and call callbacks are rooted
+ * - callbacks in GObjects methods are traced from the GObject
+ * (and same for vfuncs, which are associated with a GObject prototype)
+ */
+ should_root = scope != GI_SCOPE_TYPE_NOTIFIED || scope_object == NULL;
+ trampoline->js_function = gjs_closure_new(context,
+ JSVAL_TO_OBJECT(function),
+ g_base_info_get_name((GIBaseInfo*)callable_info),
+ should_root);
+ if (!should_root && scope_object)
+ gjs_object_associate_closure(context, scope_object, trampoline->js_function);
/* Analyze param types and directions, similarly to init_cached_function_data */
n_args = g_callable_info_get_n_args(trampoline->info);
@@ -511,12 +515,15 @@ static JSBool
gjs_fill_method_instance (JSContext *context,
JSObject *obj,
Function *function,
- GIArgument *out_arg)
+ GIArgument *out_arg,
+ gboolean *is_gobject)
{
GIBaseInfo *container = g_base_info_get_container((GIBaseInfo *) function->info);
GIInfoType type = g_base_info_get_type(container);
GType gtype = g_registered_type_info_get_g_type ((GIRegisteredTypeInfo *)container);
+ *is_gobject = FALSE;
+
switch (type) {
case GI_INFO_TYPE_STRUCT:
case GI_INFO_TYPE_BOXED:
@@ -551,6 +558,7 @@ gjs_fill_method_instance (JSContext *context,
return JS_FALSE;
out_arg->v_pointer = gjs_g_object_from_object(context, obj);
+ *is_gobject = TRUE;
break;
default:
@@ -597,6 +605,7 @@ gjs_invoke_c_function(JSContext *context,
gboolean failed, postinvoke_release_failed;
gboolean is_method;
+ gboolean is_object_method = FALSE;
GITypeInfo return_info;
GITypeTag return_tag;
jsval *return_values = NULL;
@@ -659,7 +668,8 @@ gjs_invoke_c_function(JSContext *context,
if (is_method) {
if (!gjs_fill_method_instance(context, obj,
- function, &in_arg_cvalues[0]))
+ function, &in_arg_cvalues[0],
+ &is_object_method))
return JS_FALSE;
ffi_arg_pointers[0] = &in_arg_cvalues[0];
++c_arg_pos;
@@ -763,6 +773,7 @@ gjs_invoke_c_function(JSContext *context,
value,
callable_info,
scope,
+ is_object_method ? obj : NULL,
FALSE);
closure = trampoline->closure;
g_base_info_unref(callable_info);
diff --git a/gi/function.h b/gi/function.h
index b77b5c5..92a99bd 100644
--- a/gi/function.h
+++ b/gi/function.h
@@ -42,9 +42,8 @@ typedef enum {
typedef struct {
gint ref_count;
- JSRuntime *runtime;
GICallableInfo *info;
- jsval js_function;
+ GClosure *js_function;
ffi_cif cif;
ffi_closure *closure;
GIScopeType scope;
@@ -56,6 +55,7 @@ GjsCallbackTrampoline* gjs_callback_trampoline_new(JSContext *context,
jsval function,
GICallableInfo *callable_info,
GIScopeType scope,
+ JSObject *scope_object,
gboolean is_vfunc);
void gjs_callback_trampoline_unref(GjsCallbackTrampoline *trampoline);
diff --git a/gi/object.c b/gi/object.c
index 9909e30..d565286 100644
--- a/gi/object.c
+++ b/gi/object.c
@@ -53,8 +53,9 @@ typedef struct {
JSObject *keep_alive; /* NULL if we are not added to it */
GType gtype;
- /* a list of all signal connections, used when tracing */
- GList *signals;
+ /* a list of all GClosures installed on this object (from
+ signals, trampolines and explicit GClosures), used when tracing */
+ GList *closures;
/* the GObjectClass wrapped by this JS Object (only used for
prototypes) */
@@ -67,6 +68,8 @@ typedef struct {
GClosure *closure;
} ConnectData;
+typedef void (*GClosureFunc) (GClosure*, gpointer);
+
enum {
PROP_0,
PROP_JS_CONTEXT,
@@ -111,6 +114,16 @@ gjs_is_custom_type_quark (void)
}
static GQuark
+gjs_object_priv_quark (void)
+{
+ static GQuark val = 0;
+ if (!val)
+ val = g_quark_from_static_string ("gjs::class-private");
+
+ return val;
+}
+
+static GQuark
gjs_is_custom_property_quark (void)
{
static GQuark val = 0;
@@ -1044,20 +1057,28 @@ GJS_NATIVE_CONSTRUCTOR_DECLARE(object_instance)
}
static void
-invalidate_all_signals(ObjectInstance *priv)
+closures_foreach(ObjectInstance *priv,
+ GClosureFunc func,
+ gpointer user_data)
{
GList *iter, *next;
- for (iter = priv->signals; iter; ) {
+ for (iter = priv->closures; iter; ) {
ConnectData *cd = iter->data;
next = iter->next;
- /* This will also free cd and iter, through
- the closure invalidation mechanism */
- g_closure_invalidate(cd->closure);
+ /* This could also free cd and iter, for
+ example when func is g_closure_invalidate */
+ func(cd->closure, user_data);
iter = next;
}
+}
+
+static void
+invalidate_all_closures(ObjectInstance *priv)
+{
+ closures_foreach(priv, (GClosureFunc) g_closure_invalidate, NULL);
}
static void
@@ -1065,15 +1086,9 @@ object_instance_trace(JSTracer *tracer,
JSObject *obj)
{
ObjectInstance *priv;
- GList *iter;
priv = priv_from_js(tracer->context, obj);
-
- for (iter = priv->signals; iter; iter = iter->next) {
- ConnectData *cd = iter->data;
-
- gjs_closure_trace(cd->closure, tracer);
- }
+ closures_foreach(priv, (GClosureFunc) gjs_closure_trace, tracer);
}
static void
@@ -1097,7 +1112,7 @@ object_instance_finalize(JSContext *context,
priv->info ? g_base_info_get_name((GIBaseInfo*) priv->info) : g_type_name(priv->gtype)));
if (priv->gobj) {
- invalidate_all_signals (priv);
+ invalidate_all_closures (priv);
if (G_UNLIKELY (priv->gobj->ref_count <= 0)) {
g_error("Finalizing proxy for an already freed object of type: %s.%s\n",
@@ -1158,16 +1173,47 @@ gjs_lookup_object_prototype(JSContext *context,
}
static void
-signal_connection_invalidated (gpointer user_data,
+closure_invalidated (gpointer user_data,
GClosure *closure)
{
ConnectData *connect_data = user_data;
- connect_data->obj->signals = g_list_delete_link(connect_data->obj->signals,
- connect_data->link);
+ connect_data->obj->closures = g_list_delete_link(connect_data->obj->closures,
+ connect_data->link);
g_slice_free(ConnectData, connect_data);
}
+static void
+do_associate_closure (ObjectInstance *priv,
+ GClosure *closure)
+{
+ ConnectData *connect_data;
+
+ connect_data = g_slice_new(ConnectData);
+ priv->closures = g_list_prepend(priv->closures, connect_data);
+ connect_data->obj = priv;
+ connect_data->link = priv->closures;
+ /* This is a weak reference, and will be cleared when the closure is invalidated */
+ connect_data->closure = closure;
+ g_closure_add_invalidate_notifier(closure, connect_data, closure_invalidated);
+}
+
+JSBool
+gjs_object_associate_closure (JSContext *context,
+ JSObject *object,
+ GClosure *closure)
+{
+ ObjectInstance *priv;
+
+ priv = priv_from_js(context, object);
+ if (priv == NULL)
+ return JS_FALSE;
+
+ do_associate_closure(priv, closure);
+
+ return JS_TRUE;
+}
+
static JSBool
real_connect_func(JSContext *context,
uintN argc,
@@ -1183,7 +1229,6 @@ real_connect_func(JSContext *context,
char *signal_name;
GQuark signal_detail;
jsval retval;
- ConnectData *connect_data;
JSBool ret = JS_FALSE;
if (!do_base_typecheck(context, obj, JS_TRUE))
@@ -1236,13 +1281,7 @@ real_connect_func(JSContext *context,
if (closure == NULL)
goto out;
- connect_data = g_slice_new(ConnectData);
- priv->signals = g_list_prepend(priv->signals, connect_data);
- connect_data->obj = priv;
- connect_data->link = priv->signals;
- /* This is a weak reference, and will be cleared when the closure is invalidated */
- connect_data->closure = closure;
- g_closure_add_invalidate_notifier(closure, connect_data, signal_connection_invalidated);
+ do_associate_closure(priv, closure);
id = g_signal_connect_closure(priv->gobj,
signal_name,
@@ -2156,7 +2195,7 @@ gjs_hook_up_vfunc(JSContext *cx,
method_ptr = G_STRUCT_MEMBER_P(implementor_vtable, offset);
trampoline = gjs_callback_trampoline_new(cx, OBJECT_TO_JSVAL(function), callback_info,
- GI_SCOPE_TYPE_NOTIFIED, TRUE);
+ GI_SCOPE_TYPE_NOTIFIED, object, TRUE);
*((ffi_closure **)method_ptr) = trampoline->closure;
@@ -2269,6 +2308,30 @@ gjs_object_class_init(GObjectClass *class,
G_PARAM_WRITABLE | G_PARAM_CONSTRUCT_ONLY | G_PARAM_STATIC_STRINGS));
}
+static void
+gjs_object_base_init(GObjectClass *klass,
+ gpointer user_data)
+{
+ ObjectInstance *priv;
+
+ priv = g_type_get_qdata (G_OBJECT_CLASS_TYPE(klass), gjs_object_priv_quark());
+
+ if (priv)
+ closures_foreach(priv, (GClosureFunc) g_closure_ref, NULL);
+}
+
+static void
+gjs_object_base_finalize(GObjectClass *klass,
+ gpointer user_data)
+{
+ ObjectInstance *priv;
+
+ priv = g_type_get_qdata (G_OBJECT_CLASS_TYPE(klass), gjs_object_priv_quark());
+
+ if (priv)
+ closures_foreach(priv, (GClosureFunc) g_closure_unref, NULL);
+}
+
static JSBool
gjs_register_type(JSContext *cx,
uintN argc,
@@ -2276,16 +2339,16 @@ gjs_register_type(JSContext *cx,
{
jsval *argv = JS_ARGV(cx, vp);
gchar *name;
- JSObject *parent, *constructor;
+ JSObject *parent, *constructor, *prototype;
GType instance_type, parent_type;
GTypeQuery query;
GTypeModule *type_module;
- ObjectInstance *parent_priv;
+ ObjectInstance *parent_priv, *priv;
GTypeInfo type_info = {
0, /* class_size */
- (GBaseInitFunc) NULL,
- (GBaseFinalizeFunc) NULL,
+ (GBaseInitFunc) gjs_object_base_init,
+ (GBaseFinalizeFunc) gjs_object_base_finalize,
(GClassInitFunc) gjs_object_class_init,
(GClassFinalizeFunc) NULL,
@@ -2337,15 +2400,17 @@ gjs_register_type(JSContext *cx,
name,
&type_info,
0);
-
g_free(name);
g_type_set_qdata (instance_type, gjs_is_custom_type_quark(), GINT_TO_POINTER (1));
/* create a custom JSClass */
- if (!gjs_define_object_class(cx, NULL, instance_type, &constructor, NULL))
+ if (!gjs_define_object_class(cx, NULL, instance_type, &constructor, &prototype))
return JS_FALSE;
+ priv = priv_from_js(cx, prototype);
+ g_type_set_qdata (instance_type, gjs_object_priv_quark(), priv);
+
JS_SET_RVAL(cx, vp, OBJECT_TO_JSVAL(constructor));
JS_EndRequest(cx);
diff --git a/gi/object.h b/gi/object.h
index 1957abd..b745811 100644
--- a/gi/object.h
+++ b/gi/object.h
@@ -24,7 +24,7 @@
#ifndef __GJS_OBJECT_H__
#define __GJS_OBJECT_H__
-#include <glib.h>
+#include <glib-object.h>
#include <jsapi.h>
@@ -47,6 +47,9 @@ JSBool gjs_typecheck_object (JSContext *context,
JSObject *obj,
GType expected_type,
JSBool throw);
+JSBool gjs_object_associate_closure (JSContext *context,
+ JSObject *obj,
+ GClosure *closure);
G_END_DECLS
diff --git a/gi/value.c b/gi/value.c
index 4d8de2f..68b2f67 100644
--- a/gi/value.c
+++ b/gi/value.c
@@ -124,7 +124,7 @@ closure_marshal(GClosure *closure,
}
}
- gjs_closure_invoke(closure, argc, argv, &rval);
+ gjs_closure_invoke(closure, NULL, argc, argv, &rval);
if (return_value != NULL) {
if (JSVAL_IS_VOID(rval)) {
diff --git a/modules/dbus.c b/modules/dbus.c
index 3a62025..6baf085 100644
--- a/modules/dbus.c
+++ b/modules/dbus.c
@@ -348,7 +348,7 @@ pending_notify(DBusPendingCall *pending,
}
gjs_js_push_current_message(reply);
- gjs_closure_invoke(closure, 2, &argv[0], &discard);
+ gjs_closure_invoke(closure, NULL, 2, &argv[0], &discard);
gjs_js_pop_current_message();
if (reply)
@@ -681,6 +681,7 @@ signal_handler_callback(DBusConnection *connection,
gjs_js_push_current_message(message);
gjs_closure_invoke(handler->closure,
+ NULL,
gjs_rooted_array_get_length(context, arguments),
gjs_rooted_array_get_data(context, arguments),
&ret_val);
@@ -1079,7 +1080,7 @@ on_name_acquired(DBusConnection *connection,
JS_AddValueRoot(context, &rval);
gjs_closure_invoke(owner->acquired_closure,
- argc, argv, &rval);
+ NULL, argc, argv, &rval);
JS_RemoveValueRoot(context, &argv[0]);
JS_RemoveValueRoot(context, &rval);
@@ -1118,7 +1119,7 @@ on_name_lost(DBusConnection *connection,
JS_AddValueRoot(context, &rval);
gjs_closure_invoke(owner->lost_closure,
- argc, argv, &rval);
+ NULL, argc, argv, &rval);
JS_RemoveValueRoot(context, &argv[0]);
JS_RemoveValueRoot(context, &rval);
@@ -1311,7 +1312,7 @@ on_name_appeared(DBusConnection *connection,
JS_AddValueRoot(context, &rval);
gjs_closure_invoke(watcher->appeared_closure,
- argc, argv, &rval);
+ NULL, argc, argv, &rval);
JS_RemoveValueRoot(context, &rval);
gjs_unroot_value_locations(context, argv, argc);
@@ -1354,7 +1355,7 @@ on_name_vanished(DBusConnection *connection,
JS_AddValueRoot(context, &rval);
gjs_closure_invoke(watcher->vanished_closure,
- argc, argv, &rval);
+ NULL, argc, argv, &rval);
JS_RemoveValueRoot(context, &rval);
gjs_unroot_value_locations(context, argv, argc);
diff --git a/modules/mainloop.c b/modules/mainloop.c
index 67912b2..ee32269 100644
--- a/modules/mainloop.c
+++ b/modules/mainloop.c
@@ -134,8 +134,9 @@ closure_source_func(void *data)
JS_AddValueRoot(context, &retval);
gjs_closure_invoke(closure,
- 0, NULL,
- &retval);
+ NULL,
+ 0, NULL,
+ &retval);
/* ValueToBoolean pretty much always succeeds, just as
* JavaScript always makes some sense of any value in
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
