Commit 06aa616a authored by Giovanni Campagna's avatar Giovanni Campagna
Browse files

Trace signal closures from the object instead of the context keep-alive

Implement tracing for GObjects and use it so that signal handlers
are rooted to the objects they're connected, and not the global
object. This means that if the object goes away and the only thing
referring to it is the handler function, it is recognized as a
cycle by the GC and collected, reducing memory leaks.
Other closures, as well as callback trampolines and vfuncs, are still
rooted in the usual way.

https://bugzilla.gnome.org/show_bug.cgi?id=678504
parent 1311a110
......@@ -229,6 +229,17 @@ closure_invalidated(gpointer data,
}
}
static void
closure_set_invalid(gpointer data,
GClosure *closure)
{
Closure *self = (Closure*) closure;
self->obj = NULL;
self->context = NULL;
self->runtime = NULL;
}
static void
closure_finalized(gpointer data,
GClosure *closure)
......@@ -317,10 +328,25 @@ gjs_closure_get_callable(GClosure *closure)
return c->obj;
}
void
gjs_closure_trace(GClosure *closure,
JSTracer *tracer)
{
Closure *c;
c = (Closure*) closure;
if (c->obj == NULL)
return;
JS_CALL_OBJECT_TRACER(tracer, c->obj, "signal connection");
}
GClosure*
gjs_closure_new(JSContext *context,
JSObject *callable,
const char *description)
JSObject *callable,
const char *description,
gboolean root_function)
{
Closure *c;
......@@ -343,12 +369,19 @@ gjs_closure_new(JSContext *context,
*/
g_closure_add_finalize_notifier(&c->base, NULL, closure_finalized);
gjs_keep_alive_add_global_child(context,
global_context_finalized,
c->obj,
c);
if (root_function) {
/* Fully manage closure lifetime if so asked */
gjs_keep_alive_add_global_child(context,
global_context_finalized,
c->obj,
c);
g_closure_add_invalidate_notifier(&c->base, NULL, closure_invalidated);
g_closure_add_invalidate_notifier(&c->base, NULL, closure_invalidated);
} else {
/* Only mark the closure as invalid if memory is managed
outside (i.e. by object.c for signals) */
g_closure_add_invalidate_notifier(&c->base, NULL, closure_set_invalid);
}
gjs_debug_closure("Create closure %p which calls object %p '%s'",
c, c->obj, description);
......@@ -357,4 +390,3 @@ gjs_closure_new(JSContext *context,
return &c->base;
}
......@@ -32,7 +32,8 @@ G_BEGIN_DECLS
GClosure* gjs_closure_new (JSContext *context,
JSObject *callable,
const char *description);
const char *description,
gboolean root_function);
void gjs_closure_invoke (GClosure *closure,
int argc,
jsval *argv,
......@@ -41,6 +42,9 @@ JSRuntime* gjs_closure_get_runtime (GClosure *closure);
gboolean gjs_closure_is_valid (GClosure *closure);
JSObject* gjs_closure_get_callable (GClosure *closure);
void gjs_closure_trace (GClosure *closure,
JSTracer *tracer);
G_END_DECLS
#endif /* __GJS_CLOSURE_H__ */
......@@ -34,6 +34,7 @@
#include "param.h"
#include "value.h"
#include "keep-alive.h"
#include "closure.h"
#include "gjs_gi_trace.h"
#include <gjs/gjs-module.h>
......@@ -50,8 +51,17 @@ typedef struct {
GObject *gobj; /* NULL if we are the prototype and not an instance */
JSObject *keep_alive; /* NULL if we are not added to it */
GType gtype;
/* a list of all signal connections, used when tracing */
GList *signals;
} ObjectInstance;
typedef struct {
ObjectInstance *obj;
GList *link;
GClosure *closure;
} ConnectData;
enum {
PROP_0,
PROP_JS_CONTEXT,
......@@ -1010,6 +1020,39 @@ GJS_NATIVE_CONSTRUCTOR_DECLARE(object_instance)
return ret;
}
static void
invalidate_all_signals(ObjectInstance *priv)
{
GList *iter, *next;
for (iter = priv->signals; 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);
iter = next;
}
}
static void
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);
}
}
static void
object_instance_finalize(JSContext *context,
JSObject *obj)
......@@ -1031,6 +1074,8 @@ 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);
if (G_UNLIKELY (priv->gobj->ref_count <= 0)) {
g_error("Finalizing proxy for an already freed object of type: %s.%s\n",
priv->info ? g_base_info_get_namespace((GIBaseInfo*) priv->info) : "",
......@@ -1084,6 +1129,17 @@ gjs_lookup_object_prototype(JSContext *context,
return proto;
}
static void
signal_connection_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);
g_slice_free(ConnectData, connect_data);
}
static JSBool
real_connect_func(JSContext *context,
uintN argc,
......@@ -1099,6 +1155,7 @@ real_connect_func(JSContext *context,
char *signal_name;
GQuark signal_detail;
jsval retval;
ConnectData *connect_data;
JSBool ret = JS_FALSE;
priv = priv_from_js(context, obj);
......@@ -1148,6 +1205,14 @@ 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);
id = g_signal_connect_closure(priv->gobj,
signal_name,
closure,
......@@ -1392,7 +1457,8 @@ static struct JSClass gjs_object_instance_class = {
NULL, /* We copy this class struct with multiple names */
JSCLASS_HAS_PRIVATE |
JSCLASS_NEW_RESOLVE |
JSCLASS_NEW_RESOLVE_GETS_START,
JSCLASS_NEW_RESOLVE_GETS_START |
JSCLASS_MARK_IS_TRACE,
JS_PropertyStub,
JS_PropertyStub,
object_instance_get_prop,
......@@ -1401,7 +1467,14 @@ static struct JSClass gjs_object_instance_class = {
(JSResolveOp) object_instance_new_resolve, /* needs cast since it's the new resolve signature */
JS_ConvertStub,
object_instance_finalize,
JSCLASS_NO_OPTIONAL_MEMBERS
NULL,
NULL,
NULL,
NULL,
NULL,
NULL,
JS_CLASS_TRACE(object_instance_trace),
NULL,
};
static JSBool
......
......@@ -154,7 +154,7 @@ gjs_closure_new_for_signal(JSContext *context,
{
GClosure *closure;
closure = gjs_closure_new(context, callable, description);
closure = gjs_closure_new(context, callable, description, FALSE);
g_closure_set_meta_marshal(closure, GUINT_TO_POINTER(signal_id), closure_marshal);
......@@ -168,7 +168,7 @@ gjs_closure_new_marshaled (JSContext *context,
{
GClosure *closure;
closure = gjs_closure_new(context, callable, description);
closure = gjs_closure_new(context, callable, description, TRUE);
g_closure_set_marshal(closure, closure_marshal);
......
......@@ -429,7 +429,7 @@ gjs_js_dbus_call_async(JSContext *context,
* and deal with the GC root and other issues, even though we
* won't ever marshal via GValue
*/
closure = gjs_closure_new(context, JSVAL_TO_OBJECT(argv[9]), "async call");
closure = gjs_closure_new(context, JSVAL_TO_OBJECT(argv[9]), "async call", TRUE);
if (closure == NULL) {
dbus_pending_call_unref(pending);
return JS_FALSE;
......@@ -505,7 +505,8 @@ signal_handler_new(JSContext *context,
*/
handler->closure = gjs_closure_new(context,
JSVAL_TO_OBJECT(callable),
"signal watch");
"signal watch",
TRUE);
if (handler->closure == NULL) {
g_free(handler);
return NULL;
......@@ -1208,13 +1209,13 @@ gjs_js_dbus_acquire_name(JSContext *context,
owner->bus_type = bus_type;
owner->acquired_closure =
gjs_closure_new(context, acquire_func, "acquired bus name");
gjs_closure_new(context, acquire_func, "acquired bus name", TRUE);
g_closure_ref(owner->acquired_closure);
g_closure_sink(owner->acquired_closure);
owner->lost_closure =
gjs_closure_new(context, lost_func, "lost bus name");
gjs_closure_new(context, lost_func, "lost bus name", TRUE);
g_closure_ref(owner->lost_closure);
g_closure_sink(owner->lost_closure);
......@@ -1440,13 +1441,13 @@ gjs_js_dbus_watch_name(JSContext *context,
watcher = g_slice_new0(GjsJSDBusNameWatcher);
watcher->appeared_closure =
gjs_closure_new(context, appeared_func, "service appeared");
gjs_closure_new(context, appeared_func, "service appeared", TRUE);
g_closure_ref(watcher->appeared_closure);
g_closure_sink(watcher->appeared_closure);
watcher->vanished_closure =
gjs_closure_new(context, vanished_func, "service vanished");
gjs_closure_new(context, vanished_func, "service vanished", TRUE);
g_closure_ref(watcher->vanished_closure);
g_closure_sink(watcher->vanished_closure);
......
......@@ -198,7 +198,7 @@ gjs_timeout_add(JSContext *context,
"interval", &interval, "callback", &callback))
return JS_FALSE;
closure = gjs_closure_new(context, callback, "timeout");
closure = gjs_closure_new(context, callback, "timeout", TRUE);
if (closure == NULL)
return JS_FALSE;
......@@ -241,7 +241,7 @@ gjs_timeout_add_seconds(JSContext *context,
"interval", &interval, "callback", &callback))
return JS_FALSE;
closure = gjs_closure_new(context, callback, "timeout_seconds");
closure = gjs_closure_new(context, callback, "timeout_seconds", TRUE);
if (closure == NULL)
return JS_FALSE;
......@@ -288,7 +288,7 @@ gjs_idle_add(JSContext *context,
"callback", &callback, "priority", &priority))
return JS_FALSE;
closure = gjs_closure_new(context, callback, "idle");
closure = gjs_closure_new(context, callback, "idle", TRUE);
if (closure == NULL)
return JS_FALSE;
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment