Reworking of Arjan Molenaar's (arjanmolenaar@hetnet.nl) patch from bugPYGTK_1_99_11

2002-07-09  James Henstridge  <james@daa.com.au>

	Reworking of Arjan Molenaar's (arjanmolenaar@hetnet.nl) patch from
	bug 71435.

	* gtk/libglade.override (connect_one): watch the closure.
	(connect_many): watch the closure.

	* gtk/gtk.override (_wrap_gtk_toolbar_append_item): watch closure
	for signal.
	(_wrap_gtk_toolbar_prepend_item): same here.
	(_wrap_gtk_toolbar_insert_item): same here.
	(_wrap_gtk_toolbar_insert_stock): same here.
	(_wrap_gtk_toolbar_append_element): same here.
	(_wrap_gtk_toolbar_prepend_element): same here.
	(_wrap_gtk_toolbar_insert_element): same here.

	* pygobject.h (_PyGObject_Functions): add pygobject_watch_closure
	to the list of exported functions.

	* pygobject.c (pygobject_watch_closure): new function to watch a
	closure.  We perform cyclic garbage collection on watched
	closures.  The closure will automatically be unwatched when it
	gets invalidated.
	(pygobject_traverse): traverse watched closures as well.
	(pygobject_clear): invalidate all watched closures
	(pygobject_dealloc): invalidate watched closures on dealloc too.
	(PyGObject_Type): register the invalidate handler.
	(pygobject_connect): watch the closure we connect here.
	(pygobject_connect_after): same here..
	(pygobject_connect_object): same here.
	(pygobject_connect_object_after): same here.

	* pygtype.c (pyg_closure_new): clean up closure on invalidate,
	rather than finalize (on invalidate, we break references).

	* pygobject.h (PyGObject): add closures member to store references
	to PyGClosures.

5 files changed, 111 insertions, 19 deletions

diff --git a/gobject/gobjectmodule.c b/gobject/gobjectmodule.c
index 2e6b471..33f82f0 100644
--- a/gobject/gobjectmodule.c
+++ b/gobject/gobjectmodule.c
@@ -1295,6 +1295,7 @@ struct _PyGObject_Functions pygobject_api_functions = {
   pygobject_new,
 
   pyg_closure_new,
+  pygobject_watch_closure,
   pyg_destroy_notify,
 
   pyg_type_from_object,
diff --git a/gobject/pygobject-private.h b/gobject/pygobject-private.h
index e8b768d..816615e 100644
--- a/gobject/pygobject-private.h
+++ b/gobject/pygobject-private.h
@@ -52,6 +52,14 @@ void      pyg_register_boxed_custom(GType boxed_type,
 int       pyg_value_from_pyobject(GValue *value, PyObject *obj);
 PyObject *pyg_value_as_pyobject(const GValue *value, gboolean copy_boxed);
 
+typedef struct _PyGClosure PyGClosure;
+struct _PyGClosure {
+    GClosure closure;
+    PyObject *callback;
+    PyObject *extra_args; /* tuple of extra args to pass to callback */
+    PyObject *swap_data; /* other object for gtk_signal_connect_object */
+};
+
 GClosure *pyg_closure_new(PyObject *callback, PyObject *extra_args, PyObject *swap_data);
 GClosure *pyg_signal_class_closure_get(void);
 
@@ -68,7 +76,7 @@ void          pygobject_register_class   (PyObject *dict,
 void          pygobject_register_wrapper (PyObject *self);
 PyObject *    pygobject_new              (GObject *obj);
 PyTypeObject *pygobject_lookup_class     (GType gtype);
-
+void          pygobject_watch_closure    (PyObject *self, GClosure *closure);
 
 /* from pygboxed.c */
 extern PyTypeObject PyGBoxed_Type;
diff --git a/gobject/pygobject.c b/gobject/pygobject.c
index 29280d1..30aeb38 100644
--- a/gobject/pygobject.c
+++ b/gobject/pygobject.c
@@ -127,6 +127,7 @@ pygobject_new(GObject *obj)
     self->hasref = FALSE;
     self->inst_dict = NULL;
     self->weakreflist = NULL;
+    self->closures = NULL;
     /* save wrapper pointer so we can access it later */
     g_object_set_qdata(obj, pygobject_wrapper_key, self);
 
@@ -135,12 +136,36 @@ pygobject_new(GObject *obj)
     return (PyObject *)self;
 }
 
+static void
+pygobject_unwatch_closure(gpointer data, GClosure *closure)
+{
+    PyGObject *self = (PyGObject *)data;
+
+    self->closures = g_slist_remove (self->closures, closure);
+}
+
+void
+pygobject_watch_closure(PyObject *self, GClosure *closure)
+{
+    PyGObject *gself;
+
+    g_return_if_fail(self != NULL);
+    g_return_if_fail(PyObject_TypeCheck(self, &PyGObject_Type));
+    g_return_if_fail(closure != NULL);
+    g_return_if_fail(g_slist_find(((PyGObject *)self)->closures, closure) == NULL);
+
+    gself = (PyGObject *)self;
+    gself->closures = g_slist_prepend(gself->closures, closure);
+    g_closure_add_invalidate_notifier(closure,self, pygobject_unwatch_closure);
+}
+
 /* -------------- PyGObject behaviour ----------------- */
 
 static void
 pygobject_dealloc(PyGObject *self)
 {
     GObject *obj = self->obj;
+    GSList *tmp;
 
     if (!pygobject_ownedref_key)
 	pygobject_ownedref_key =
@@ -182,6 +207,17 @@ pygobject_dealloc(PyGObject *self)
     }
     self->inst_dict = NULL;
 
+    tmp = self->closures;
+    while (tmp) {
+	GClosure *closure = tmp->data;
+
+	/* we get next item first, because the current link gets
+	 * invalidated by pygobject_unwatch_closure */
+	tmp = tmp->next;
+	g_closure_invalidate(closure);
+    }
+    self->closures = NULL;
+
     /* the following causes problems with subclassed types */
     /*self->ob_type->tp_free((PyObject *)self); */
     PyObject_GC_Del(self);
@@ -217,8 +253,44 @@ pygobject_repr(PyGObject *self)
 static int
 pygobject_traverse(PyGObject *self, visitproc visit, void *arg)
 {
-    if (self->inst_dict)
-	return visit(self->inst_dict, arg);
+    int ret = 0;
+    GSList *tmp;
+
+    if (self->inst_dict) ret = visit(self->inst_dict, arg);
+    if (ret != 0) return ret;
+
+    for (tmp = self->closures; tmp != NULL; tmp = tmp->next) {
+	PyGClosure *closure = tmp->data;
+
+	if (closure->callback) ret = visit(closure->callback, arg);
+	if (ret != 0) return ret;
+
+	if (closure->extra_args) ret = visit(closure->extra_args, arg);
+	if (ret != 0) return ret;
+
+	if (closure->swap_data) ret = visit(closure->swap_data, arg);
+	if (ret != 0) return ret;
+    }
+    return 0;
+}
+
+static int
+pygobject_clear(PyGObject *self)
+{
+    GSList *tmp;
+
+    tmp = self->closures;
+    while (tmp) {
+	GClosure *closure = tmp->data;
+
+	/* we get next item first, because the current link gets
+	 * invalidated by pygobject_unwatch_closure */
+	tmp = tmp->next;
+	g_closure_invalidate(closure);
+    }
+    if (self->closures != NULL)
+	g_message("invalidated all closures, but self->closures != NULL !");
+
     return 0;
 }
 
@@ -228,6 +300,7 @@ pygobject_free(PyObject *op)
     PyObject_GC_Del(op);
 }
 
+
 /* ---------------- PyGObject methods ----------------- */
 
 static void
@@ -451,6 +524,7 @@ pygobject_connect(PyGObject *self, PyObject *args)
     gchar *name;
     guint handlerid, sigid, len;
     GQuark detail = 0;
+    GClosure *closure;
 
     len = PyTuple_Size(args);
     if (len < 2) {
@@ -476,8 +550,10 @@ pygobject_connect(PyGObject *self, PyObject *args)
     extra_args = PySequence_GetSlice(args, 2, len);
     if (extra_args == NULL)
 	return NULL;
+    closure = pyg_closure_new(callback, extra_args, NULL);
+    pygobject_watch_closure((PyObject *)self, closure);
     handlerid = g_signal_connect_closure_by_id(self->obj, sigid, detail,
-			pyg_closure_new(callback, extra_args, NULL), FALSE);
+					       closure, FALSE);
     Py_DECREF(extra_args);
     return PyInt_FromLong(handlerid);
 }
@@ -489,6 +565,7 @@ pygobject_connect_after(PyGObject *self, PyObject *args)
     gchar *name;
     guint handlerid, sigid, len;
     GQuark detail;
+    GClosure *closure;
 
     len = PyTuple_Size(args);
     if (len < 2) {
@@ -515,8 +592,10 @@ pygobject_connect_after(PyGObject *self, PyObject *args)
     extra_args = PySequence_GetSlice(args, 2, len);
     if (extra_args == NULL)
 	return NULL;
+    closure = pyg_closure_new(callback, extra_args, NULL);
+    pygobject_watch_closure((PyObject *)self, closure);
     handlerid = g_signal_connect_closure_by_id(self->obj, sigid, detail,
-			pyg_closure_new(callback, extra_args, NULL), TRUE);
+					       closure, TRUE);
     Py_DECREF(extra_args);
     return PyInt_FromLong(handlerid);
 }
@@ -528,6 +607,7 @@ pygobject_connect_object(PyGObject *self, PyObject *args)
     gchar *name;
     guint handlerid, sigid, len;
     GQuark detail;
+    GClosure *closure;
 
     len = PyTuple_Size(args);
     if (len < 3) {
@@ -554,8 +634,10 @@ pygobject_connect_object(PyGObject *self, PyObject *args)
     extra_args = PySequence_GetSlice(args, 3, len);
     if (extra_args == NULL)
 	return NULL;
+    closure = pyg_closure_new(callback, extra_args, NULL);
+    pygobject_watch_closure((PyObject *)self, closure);
     handlerid = g_signal_connect_closure_by_id(self->obj, sigid, detail,
-			pyg_closure_new(callback, extra_args, object), FALSE);
+					       closure, FALSE);
     Py_DECREF(extra_args);
     return PyInt_FromLong(handlerid);
 }
@@ -567,6 +649,7 @@ pygobject_connect_object_after(PyGObject *self, PyObject *args)
     gchar *name;
     guint handlerid, sigid, len;
     GQuark detail;
+    GClosure *closure;
 
     len = PyTuple_Size(args);
     if (len < 3) {
@@ -593,8 +676,10 @@ pygobject_connect_object_after(PyGObject *self, PyObject *args)
     extra_args = PySequence_GetSlice(args, 3, len);
     if (extra_args == NULL)
 	return NULL;
+    closure = pyg_closure_new(callback, extra_args, NULL);
+    pygobject_watch_closure((PyObject *)self, closure);
     handlerid = g_signal_connect_closure_by_id(self->obj, sigid, detail,
-			pyg_closure_new(callback, extra_args, object), TRUE);
+					       closure, TRUE);
     Py_DECREF(extra_args);
     return PyInt_FromLong(handlerid);
 }
@@ -875,7 +960,7 @@ PyTypeObject PyGObject_Type = {
     	Py_TPFLAGS_HAVE_GC,		/* tp_flags */
     NULL, /* Documentation string */
     (traverseproc)pygobject_traverse,	/* tp_traverse */
-    (inquiry)0,				/* tp_clear */
+    (inquiry)pygobject_clear,		/* tp_clear */
     (richcmpfunc)0,			/* tp_richcompare */
     offsetof(PyGObject, weakreflist),	/* tp_weaklistoffset */
     (getiterfunc)0,			/* tp_iter */
diff --git a/gobject/pygobject.h b/gobject/pygobject.h
index 5cc4f49..90bfba0 100644
--- a/gobject/pygobject.h
+++ b/gobject/pygobject.h
@@ -13,6 +13,7 @@ typedef struct {
     gboolean hasref;     /* the GObject owns this reference */
     PyObject *inst_dict; /* the instance dictionary -- must be last */
     PyObject *weakreflist; /* list of weak references */
+    GSList *closures;
 } PyGObject;
 
 #define pygobject_get(v) (((PyGObject *)(v))->obj)
@@ -49,6 +50,7 @@ struct _PyGObject_Functions {
 
     GClosure *(* closure_new)(PyObject *callback, PyObject *extra_args,
 			      PyObject *swap_data);
+    void      (* object_watch_closure)(PyObject *self, GClosure *closure);
     GDestroyNotify destroy_notify;
 
     GType (* type_from_object)(PyObject *obj);
@@ -107,6 +109,7 @@ struct _PyGObject_Functions *_PyGObject_API;
 #define pygobject_lookup_class     (_PyGObject_API->lookup_class)
 #define pygobject_new              (_PyGObject_API->newgobj)
 #define pyg_closure_new            (_PyGObject_API->closure_new)
+#define pygobject_watch_closure    (_PyGObject_API->object_watch_closure)
 #define pyg_destroy_notify         (_PyGObject_API->destroy_notify)
 #define pyg_type_from_object       (_PyGObject_API->type_from_object)
 #define pyg_type_wrapper_new       (_PyGObject_API->type_wrapper_new)
diff --git a/gobject/pygtype.c b/gobject/pygtype.c
index 9db7990..09edeb1 100644
--- a/gobject/pygtype.c
+++ b/gobject/pygtype.c
@@ -611,23 +611,18 @@ pyg_value_as_pyobject(const GValue *value, gboolean copy_boxed)
 
 /* -------------- PyGClosure ----------------- */
 
-typedef struct _PyGClosure PyGClosure;
-struct _PyGClosure {
-    GClosure closure;
-    PyObject *callback;
-    PyObject *extra_args; /* tuple of extra args to pass to callback */
-    PyObject *swap_data; /* other object for gtk_signal_connect_object */
-};
-
 static void
-pyg_closure_destroy(gpointer data, GClosure *closure)
+pyg_closure_invalidate(gpointer data, GClosure *closure)
 {
     PyGClosure *pc = (PyGClosure *)closure;
 
     pyg_block_threads();
-    Py_DECREF(pc->callback);
+    Py_XDECREF(pc->callback);
     Py_XDECREF(pc->extra_args);
     Py_XDECREF(pc->swap_data);
+    pc->callback = NULL;
+    pc->extra_args = NULL;
+    pc->swap_data = NULL;
     pyg_unblock_threads();
 }
 
@@ -693,7 +688,7 @@ pyg_closure_new(PyObject *callback, PyObject *extra_args, PyObject *swap_data)
 
     g_return_val_if_fail(callback != NULL, NULL);
     closure = g_closure_new_simple(sizeof(PyGClosure), NULL);
-    g_closure_add_finalize_notifier(closure, NULL, pyg_closure_destroy);
+    g_closure_add_invalidate_notifier(closure, NULL, pyg_closure_invalidate);
     g_closure_set_marshal(closure, pyg_closure_marshal);
     Py_INCREF(callback);
     ((PyGClosure *)closure)->callback = callback;