gjs r138 - trunk/gi

[otaylor svn gnome org]

Author: otaylor
Date: Thu Jan  8 21:32:27 2009
New Revision: 138
URL: http://svn.gnome.org/viewvc/gjs?rev=138&view=rev

Log:
Don't keep an extra reference to closures

There's no need to hold an extra reference to closures ourselves; the
user of the closure (the signal system say), will keep the closure
referenced as long as it can be invoked.

Since invalidation notifiers are typically only called when the closure
is finalized, this was causing almost all closures (and the referenced
JS objects) to be leaked, since a referenced closure would never be
finalized or invalidated.

http://bugzilla.gnome.org/show_bug.cgi?id=567078

Modified:
   trunk/gi/closure.c

Modified: trunk/gi/closure.c
==============================================================================
--- trunk/gi/closure.c	(original)
+++ trunk/gi/closure.c	Thu Jan  8 21:32:27 2009
@@ -106,9 +106,6 @@
 
         invalidate_js_pointers(c);
     }
-
-    /* The "Keep Alive" (garbage collector) owns one reference. */
-    g_closure_unref(&c->base);
 }
 
 
@@ -137,8 +134,14 @@
     invalidate_js_pointers(c);
 }
 
-/* Invalidation is like "dispose" - it happens on signal disconnect,
- * is guaranteed to happen at finalize, but may happen before finalize
+/* Invalidation is like "dispose" - it is guaranteed to happen at
+ * finalize, but may happen before finalize. Normally, g_closure_invalidate()
+ * is called when the "target" of the closure becomes invalid, so that the
+ * source (the signal connection, say can be removed.) The usage above
+ * in invalidate_js_pointers() is typical. Since the target of the closure
+ * is under our control, it's unlikely that g_closure_invalidate() will ever
+ * be called by anyone else, but in case it ever does, it's slightly better
+ * to remove the "keep alive" here rather than in the finalize notifier.
  */
 static void
 closure_invalidated(gpointer data,
@@ -161,12 +164,6 @@
         c->obj = NULL;
         c->context = NULL;
         c->runtime = NULL;
-
-        /* The "Keep Alive" (garbage collector) owns one reference,
-         * since we removed ourselves from the keep-alive we'll
-         * never be collected so drop the ref here
-         */
-        g_closure_unref(&c->base);
     }
 }
 
@@ -309,9 +306,6 @@
                                     c->obj,
                                     c);
 
-    /* The "Keep Alive" (garbage collector) owns one reference. */
-    g_closure_ref(&c->base);
-
     g_closure_add_invalidate_notifier(&c->base, NULL, closure_invalidated);
 
     gjs_debug(GJS_DEBUG_GCLOSURE,