Bug 320428 – Break PyGObject<->GObject reference cycle

svn path=/trunk/; revision=642

7 files changed, 631 insertions, 109 deletions

diff --git a/ChangeLog b/ChangeLog
index b11738f..03f30f6 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,24 @@
+2007-04-14 Gustavo J. A. M. Carneiro  <gjc@gnome.org>
+
+	* gobject/gobjectmodule.c (init_gobject):
+	* gobject/pygobject-private.h:
+	* gobject/pygobject.c (pygobject_data_free, pygobject_data_new),
+	(pygobject_get_inst_data, pyg_toggle_notify),
+	(pygobject_switch_to_toggle_ref),
+	(pygobject_register_wrapper_full, pygobject_register_wrapper),
+	(pygobject_new_full, pygobject_unwatch_closure),
+	(pygobject_watch_closure, pygobject_dealloc, pygobject_repr),
+	(pygobject_traverse, pygobject_clear, pygobject_weak_ref),
+	(pygobject_setattro, pygobject_weak_ref_traverse),
+	(pygobject_weak_ref_notify, pygobject_weak_ref_clear),
+	(pygobject_weak_ref_dealloc, pygobject_weak_ref_new),
+	(pygobject_weak_ref_unref, pygobject_weak_ref_call):
+	* gobject/pygobject.h:
+	* gobject/pygtype.c (gclosure_from_pyfunc):
+	* tests/test_subtype.py:
+	Bug #320428: Break PyGObject<->GObject reference cycle (patch
+	v7.1; thanks John Ehresman for the help with this patch).
+
 2007-03-17  Gustavo J. A. M. Carneiro  <gjc@gnome.org>
 
 	* gobject/pygobject.c (pygobject_emit): Fix %ld vs int warning.
diff --git a/gobject/gobjectmodule.c b/gobject/gobjectmodule.c
index ceda3a5..48ca598 100644
--- a/gobject/gobjectmodule.c
+++ b/gobject/gobjectmodule.c
@@ -45,6 +45,7 @@ GQuark pygenum_class_key;
 GQuark pygflags_class_key;
 GQuark pygpointer_class_key;
 GQuark pygobject_has_updated_constructor_key;
+GQuark pygobject_instance_data_key;
 
 
 
@@ -3371,6 +3372,7 @@ init_gobject(void)
     pygpointer_class_key     = g_quark_from_static_string("PyGPointer::class");
     pygobject_has_updated_constructor_key =\
         g_quark_from_static_string("PyGObject::has-updated-constructor");
+    pygobject_instance_data_key = g_quark_from_static_string("PyGObject::instance-data");
 
     REGISTER_TYPE(d, PyGTypeWrapper_Type, "GType");
 
@@ -3425,6 +3427,9 @@ init_gobject(void)
     REGISTER_TYPE(d, PyGTimeout_Type, "Timeout");
     REGISTER_TYPE(d, PyGPollFD_Type, "PollFD");
 
+    PyType_Ready(&PyGObjectWeakRef_Type);
+    PyDict_SetItemString(d, "GObjectWeakRef", (PyObject *) &PyGObjectWeakRef_Type);
+
     REGISTER_TYPE(d, PyGOptionContext_Type, "OptionContext");
     REGISTER_TYPE(d, PyGOptionGroup_Type, "OptionGroup");
 
diff --git a/gobject/pygobject-private.h b/gobject/pygobject-private.h
index ebf107d..45e431c 100644
--- a/gobject/pygobject-private.h
+++ b/gobject/pygobject-private.h
@@ -59,6 +59,17 @@ extern struct _PyGObject_Functions pygobject_api_functions;
     } G_STMT_END
 
 
+#ifndef Py_CLEAR /* since Python 2.4 */
+# define Py_CLEAR(op)				\
+        do {                            	\
+                if (op) {			\
+                        PyObject *tmp = (PyObject *)(op);	\
+                        (op) = NULL;		\
+                        Py_DECREF(tmp);		\
+                }				\
+        } while (0)
+#endif
+
 extern GType PY_TYPE_OBJECT;
 
 extern GQuark pygboxed_type_key;
@@ -72,7 +83,9 @@ extern GQuark pygobject_class_key;
 extern GQuark pygobject_wrapper_key;
 extern GQuark pygpointer_class_key;
 extern GQuark pygobject_has_updated_constructor_key;
+extern GQuark pygobject_instance_data_key;
 
+void     pygobject_data_free  (PyGObjectData *data);
 void     pyg_destroy_notify   (gpointer     user_data);
 gboolean pyg_handler_marshal  (gpointer     user_data);
 gboolean pyg_error_check      (GError     **error);
@@ -130,7 +143,6 @@ extern PyTypeObject PyGInterface_Type;
 extern PyTypeObject PyGProps_Type;
 extern PyTypeObject PyGPropsDescr_Type;
 extern PyTypeObject PyGPropsIter_Type;
-
 void          pygobject_register_class   (PyObject *dict,
 					  const gchar *type_name,
 					  GType gtype, PyTypeObject *type,
@@ -237,6 +249,11 @@ typedef struct
     PyObject *fd_obj;
 } PyGPollFD;
 
+  /* Data that belongs to the GObject instance, not the Python wrapper */
+struct _PyGObjectData {
+    PyTypeObject *type; /* wrapper type for this instance */
+    GSList *closures;
+};
 
 /* pygoption.c */
 extern PyTypeObject PyGOptionContext_Type;
@@ -268,4 +285,15 @@ void pyg_type_register_custom_callback(const gchar *type_name,
 PyTypeObject * pyg_type_get_custom(const gchar *name);
 GType _pyg_type_from_name(const gchar *name);
 
+/* pygobject.c */
+extern PyTypeObject PyGObjectWeakRef_Type;
+
+static inline PyGObjectData *
+pyg_object_peek_inst_data(GObject *obj)
+{
+    return ((PyGObjectData *) 
+            g_object_get_qdata(obj, pygobject_instance_data_key));
+}
+
+
 #endif
diff --git a/gobject/pygobject.c b/gobject/pygobject.c
index c2ffa3c..dd4cc36 100644
--- a/gobject/pygobject.c
+++ b/gobject/pygobject.c
@@ -25,9 +25,72 @@ static void pygobject_dealloc(PyGObject *self);
 static int  pygobject_traverse(PyGObject *self, visitproc visit, void *arg);
 static int  pygobject_clear(PyGObject *self);
 static PyObject * pyg_type_get_bases(GType gtype);
+static inline int pygobject_clear(PyGObject *self);
+static PyObject * pygobject_weak_ref_new(GObject *obj, PyObject *callback, PyObject *user_data);
+static inline PyGObjectData * pyg_object_peek_inst_data(GObject *obj);
+static PyObject * pygobject_weak_ref_new(GObject *obj, PyObject *callback, PyObject *user_data);
 
 /* -------------- class <-> wrapper manipulation --------------- */
 
+void
+pygobject_data_free(PyGObjectData *data)
+{
+    PyGILState_STATE state = pyg_gil_state_ensure();
+    GSList *closures, *tmp;
+    Py_DECREF(data->type);
+    tmp = closures = data->closures;
+#ifndef NDEBUG
+    data->closures = NULL;
+    data->type = NULL;
+#endif
+    pyg_begin_allow_threads;
+    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);
+    }
+    pyg_end_allow_threads;
+ 
+    if (data->closures != NULL)
+ 	g_warning("invalidated all closures, but data->closures != NULL !");
+
+    g_free(data);
+    pyg_gil_state_release(state);
+}
+
+static inline PyGObjectData *
+pygobject_data_new(void)
+{
+    PyGObjectData *data;
+    data = g_new0(PyGObjectData, 1);
+    return data;
+}
+
+static inline PyGObjectData *
+pygobject_get_inst_data(PyGObject *self)
+{
+    PyGObjectData *inst_data;
+
+    if (G_UNLIKELY(!self->obj))
+        return NULL;
+    inst_data = g_object_get_qdata(self->obj, pygobject_instance_data_key);
+    if (inst_data == NULL)
+    {
+        inst_data = pygobject_data_new();
+
+        inst_data->type = ((PyObject *)self)->ob_type;
+        Py_INCREF((PyObject *) inst_data->type);
+
+        g_object_set_qdata_full(self->obj, pygobject_instance_data_key,
+                                inst_data, (GDestroyNotify) pygobject_data_free);
+    }
+    return inst_data;
+}
+
+
 typedef struct {
     GType type;
     void (* sinkfunc)(GObject *object);
@@ -560,6 +623,38 @@ pygobject_register_class(PyObject *dict, const gchar *type_name,
     PyDict_SetItemString(dict, (char *)class_name, (PyObject *)type);
 }
 
+static void
+pyg_toggle_notify (gpointer data, GObject *object, gboolean is_last_ref)
+{
+    PyGObject *self = (PyGObject*) data;
+    PyGILState_STATE state;
+
+    state = pyg_gil_state_ensure();
+
+    if (is_last_ref)
+	Py_DECREF(self);
+    else
+        Py_INCREF(self);
+
+    pyg_gil_state_release(state);
+}
+
+  /* Called when the inst_dict is first created; switches the 
+     reference counting strategy to start using toggle ref to keep the
+     wrapper alive while the GObject lives.  In contrast, while
+     inst_dict was NULL the python wrapper is allowed to die at
+     will and is recreated on demand. */
+static inline void
+pygobject_switch_to_toggle_ref(PyGObject *self)
+{
+    g_assert(self->obj->ref_count >= 1);
+      /* Note that add_toggle_ref will never immediately call back into 
+         pyg_toggle_notify */
+    Py_INCREF((PyObject *) self);
+    g_object_add_toggle_ref(self->obj, pyg_toggle_notify, self);
+    g_object_unref(self->obj);
+}
+
 /**
  * pygobject_register_wrapper:
  * @self: the wrapper instance
@@ -567,18 +662,27 @@ pygobject_register_class(PyObject *dict, const gchar *type_name,
  * In the constructor of PyGTK wrappers, this function should be
  * called after setting the obj member.  It will tie the wrapper
  * instance to the GObject so that the same wrapper instance will
- * always be used for this GObject instance.  It will also sink any
+ * always be used for this GObject instance.  It may also sink any
  * floating references on the GObject.
  */
+static inline void
+pygobject_register_wrapper_full(PyGObject *self, gboolean sink)
+{
+    GObject *obj = self->obj;
+
+    if (sink)
+        pygobject_sink(obj);
+    g_assert(obj->ref_count >= 1);
+      /* save wrapper pointer so we can access it later */
+    g_object_set_qdata_full(obj, pygobject_wrapper_key, self, NULL);
+    if (self->inst_dict)
+        pygobject_switch_to_toggle_ref(self);
+}
+
 void
 pygobject_register_wrapper(PyObject *self)
 {
-    GObject *obj = ((PyGObject *)self)->obj;
-
-    pygobject_sink(obj);
-    Py_INCREF(self);
-    g_object_set_qdata_full(obj, pygobject_wrapper_key, self,
-			    pyg_destroy_notify);
+    pygobject_register_wrapper_full((PyGObject *)self, TRUE);
 }
 
 static PyObject *
@@ -786,11 +890,18 @@ pygobject_new_full(GObject *obj, gboolean sink, gpointer g_class)
 	Py_INCREF(self);
     } else {
 	/* create wrapper */
-	PyTypeObject *tp;
-        if (g_class)
-            tp = pygobject_lookup_class(G_OBJECT_CLASS_TYPE(g_class));
-        else
-            tp = pygobject_lookup_class(G_OBJECT_TYPE(obj));
+        PyGObjectData *inst_data = pyg_object_peek_inst_data(obj);
+ 	PyTypeObject *tp;
+        if (inst_data)
+            tp = inst_data->type;
+        else {
+            if (g_class)
+                tp = pygobject_lookup_class(G_OBJECT_CLASS_TYPE(g_class));
+            else
+                tp = pygobject_lookup_class(G_OBJECT_TYPE(obj));
+        }
+        g_assert(tp != NULL);
+        
         /* need to bump type refcount if created with
            pygobject_new_with_interfaces(). fixes bug #141042 */
         if (tp->tp_flags & Py_TPFLAGS_HEAPTYPE)
@@ -798,18 +909,11 @@ pygobject_new_full(GObject *obj, gboolean sink, gpointer g_class)
 	self = PyObject_GC_New(PyGObject, tp);
 	if (self == NULL)
 	    return NULL;
-	self->obj = g_object_ref(obj);
-        if (sink)
-            pygobject_sink(self->obj);
-
-	self->inst_dict = NULL;
+        self->inst_dict = NULL;
 	self->weakreflist = NULL;
-	self->closures = NULL;
-	/* save wrapper pointer so we can access it later */
-	Py_INCREF(self);
-	g_object_set_qdata_full(obj, pygobject_wrapper_key, self,
-				pyg_destroy_notify);
-
+	self->obj = obj;
+	g_object_ref(obj);
+	pygobject_register_wrapper_full(self, sink);
 	PyObject_GC_Track((PyObject *)self);
     }
 
@@ -826,9 +930,9 @@ pygobject_new(GObject *obj)
 static void
 pygobject_unwatch_closure(gpointer data, GClosure *closure)
 {
-    PyGObject *self = (PyGObject *)data;
+    PyGObjectData *inst_data = data;
 
-    self->closures = g_slist_remove (self->closures, closure);
+    inst_data->closures = g_slist_remove (inst_data->closures, closure);
 }
 
 /**
@@ -847,15 +951,17 @@ void
 pygobject_watch_closure(PyObject *self, GClosure *closure)
 {
     PyGObject *gself;
+    PyGObjectData *data;
 
     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);
+    data = pygobject_get_inst_data(gself);
+    g_return_if_fail(g_slist_find(data->closures, closure) == NULL);
+    data->closures = g_slist_prepend(data->closures, closure);
+    g_closure_add_invalidate_notifier(closure, data, pygobject_unwatch_closure);
 }
 
 /* -------------- PyGObject behaviour ----------------- */
@@ -863,35 +969,13 @@ pygobject_watch_closure(PyObject *self, GClosure *closure)
 static void
 pygobject_dealloc(PyGObject *self)
 {
-    GSList *tmp;
-
     PyObject_ClearWeakRefs((PyObject *)self);
-
     PyObject_GC_UnTrack((PyObject *)self);
-
-    if (self->obj) {
-	g_object_unref(self->obj);
-    }
-    self->obj = NULL;
-
-    if (self->inst_dict) {
-	Py_DECREF(self->inst_dict);
-    }
-    self->inst_dict = NULL;
-
-    pyg_begin_allow_threads;
-    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;
-    pyg_end_allow_threads;
-
+      /* this forces inst_data->type to be updated, which could prove
+       * important if a new wrapper has to be created and it is of a
+       * unregistered type */
+    pygobject_get_inst_data(self);
+    pygobject_clear(self);
     /* the following causes problems with subclassed types */
     /* self->ob_type->tp_free((PyObject *)self); */
     PyObject_GC_Del(self);
@@ -917,72 +1001,55 @@ pygobject_repr(PyGObject *self)
     gchar buf[256];
 
     g_snprintf(buf, sizeof(buf),
-	       "<%s object (%s) at 0x%lx>",
+	       "<%s object at 0x%lx (%s at 0x%lx)>",
 	       self->ob_type->tp_name,
+	       (long)self,
 	       self->obj ? G_OBJECT_TYPE_NAME(self->obj) : "uninitialized",
-	       (long)self);
+               (long)self->obj);
     return PyString_FromString(buf);
 }
 
+
 static int
 pygobject_traverse(PyGObject *self, visitproc visit, void *arg)
 {
     int ret = 0;
     GSList *tmp;
+    PyGObjectData *data = pygobject_get_inst_data(self);
 
     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 (data) {
 
-	if (closure->callback) ret = visit(closure->callback, arg);
-	if (ret != 0) return ret;
+        for (tmp = data->closures; tmp != NULL; tmp = tmp->next) {
+            PyGClosure *closure = tmp->data;
 
-	if (closure->extra_args) ret = visit(closure->extra_args, arg);
-	if (ret != 0) return ret;
+            if (closure->callback) ret = visit(closure->callback, arg);
+            if (ret != 0) return ret;
 
-	if (closure->swap_data) ret = visit(closure->swap_data, arg);
-	if (ret != 0) return ret;
-    }
-
-    if (self->obj && self->obj->ref_count == 1)
-	ret = visit((PyObject *)self, arg);
-    if (ret != 0) return ret;
+            if (closure->extra_args) ret = visit(closure->extra_args, arg);
+            if (ret != 0) return ret;
 
-    return 0;
+            if (closure->swap_data) ret = visit(closure->swap_data, arg);
+            if (ret != 0) return ret;
+        }
+    }
+    return ret;
 }
 
-static int
+static inline int
 pygobject_clear(PyGObject *self)
 {
-    GSList *tmp;
-
-    if (self->inst_dict) {
-	Py_DECREF(self->inst_dict);
-    }
-    self->inst_dict = NULL;
-
-    pyg_begin_allow_threads;
-    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);
-    }
-    pyg_end_allow_threads;
-
-    if (self->closures != NULL)
-	g_warning("invalidated all closures, but self->closures != NULL !");
-
     if (self->obj) {
-	g_object_unref(self->obj);
+        g_object_set_qdata_full(self->obj, pygobject_wrapper_key, NULL, NULL);
+        if (self->inst_dict)
+            g_object_remove_toggle_ref(self->obj, pyg_toggle_notify, self);
+        else
+            g_object_unref(self->obj);
+        self->obj = NULL;
     }
-    self->obj = NULL;
-
+    Py_CLEAR(self->inst_dict);
     return 0;
 }
 
@@ -1667,6 +1734,26 @@ pygobject_chain_from_overridden(PyGObject *self, PyObject *args)
     return py_ret;
 }
 
+
+static PyObject *
+pygobject_weak_ref(PyGObject *self, PyObject *args)
+{
+    int len;
+    PyObject *callback = NULL, *user_data = NULL;
+    PyObject *retval;
+
+    CHECK_GOBJECT(self);
+
+    if ((len = PySequence_Length(args)) >= 1) {
+        callback = PySequence_ITEM(args, 0);
+        user_data = PySequence_GetSlice(args, 1, len);
+    }
+    retval = pygobject_weak_ref_new(self->obj, callback, user_data);
+    Py_XDECREF(callback);
+    Py_XDECREF(user_data);
+    return retval;
+}
+
 static PyObject *
 pygobject_disconnect_by_func(PyGObject *self, PyObject *args)
 {
@@ -1789,9 +1876,11 @@ static PyMethodDef pygobject_methods[] = {
     { "stop_emission", (PyCFunction)pygobject_stop_emission, METH_VARARGS },
     { "emit_stop_by_name", (PyCFunction)pygobject_stop_emission,METH_VARARGS },
     { "chain", (PyCFunction)pygobject_chain_from_overridden,METH_VARARGS },
+    { "weak_ref", (PyCFunction)pygobject_weak_ref, METH_VARARGS },
     { NULL, NULL, 0 }
 };
 
+
 static PyObject *
 pygobject_get_dict(PyGObject *self, void *closure)
 {
@@ -1810,6 +1899,20 @@ pygobject_get_refcount(PyGObject *self, void *closure)
     return PyInt_FromLong(self->obj->ref_count);
 }
 
+static int
+pygobject_setattro(PyObject *self, PyObject *name, PyObject *value)
+{
+    int res;
+    PyGObject *gself = (PyGObject *) self;
+    if (gself->inst_dict == NULL) {
+        if (G_LIKELY(gself->obj))
+            pygobject_switch_to_toggle_ref(gself);
+    }
+      /* call parent type's setattro */
+    res = PyGObject_Type.tp_base->tp_setattro(self, name, value);
+    return res;
+}
+
 static PyGetSetDef pygobject_getsets[] = {
     { "__dict__", (getter)pygobject_get_dict, (setter)0 },
     { "__grefcount__", (getter)pygobject_get_refcount, (setter)0, },
@@ -1836,7 +1939,7 @@ PyTypeObject PyGObject_Type = {
     (ternaryfunc)0,			/* tp_call */
     (reprfunc)0,			/* tp_str */
     (getattrofunc)0,			/* tp_getattro */
-    (setattrofunc)0,			/* tp_setattro */
+    (setattrofunc)pygobject_setattro,   /* tp_setattro */
     0,					/* tp_as_buffer */
     Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE |
     	Py_TPFLAGS_HAVE_GC,		/* tp_flags */
@@ -1864,3 +1967,176 @@ PyTypeObject PyGObject_Type = {
 };
 
 
+  /* ------------------------------------ */
+  /* ****** GObject weak reference ****** */
+  /* ------------------------------------ */
+
+typedef struct {
+    PyObject_HEAD
+    GObject *obj;
+    PyObject *callback;
+    PyObject *user_data;
+    gboolean have_floating_ref;
+} PyGObjectWeakRef;
+
+static int
+pygobject_weak_ref_traverse(PyGObjectWeakRef *self, visitproc visit, void *arg)
+{
+    if (self->callback && visit(self->callback, arg) < 0)
+        return -1;
+    if (self->user_data && visit(self->user_data, arg) < 0)
+        return -1;
+    return 0;
+}
+
+static void
+pygobject_weak_ref_notify(PyGObjectWeakRef *self, GObject *dummy)
+{
+    self->obj = NULL;
+    if (self->callback) {
+        PyObject *retval;
+        PyGILState_STATE state = pyg_gil_state_ensure();
+        retval = PyObject_Call(self->callback, self->user_data, NULL);
+        if (retval) {
+            if (retval != Py_None)
+                PyErr_Format(PyExc_TypeError,
+                             "GObject weak notify callback returned a value"
+                             " of type %s, should return None",
+                             retval->ob_type->tp_name);
+            Py_DECREF(retval);
+            PyErr_Print();
+        } else
+            PyErr_Print();
+        Py_CLEAR(self->callback);
+        Py_CLEAR(self->user_data);
+        if (self->have_floating_ref) {
+            self->have_floating_ref = FALSE;
+            Py_DECREF((PyObject *) self);
+        }
+        pyg_gil_state_release(state);
+    }
+}
+
+static inline int
+pygobject_weak_ref_clear(PyGObjectWeakRef *self)
+{
+    Py_CLEAR(self->callback);
+    Py_CLEAR(self->user_data);
+    if (self->obj) {
+        g_object_weak_unref(self->obj, (GWeakNotify) pygobject_weak_ref_notify, self);
+        self->obj = NULL;
+    }
+    return 0;
+}
+
+static void
+pygobject_weak_ref_dealloc(PyGObjectWeakRef *self)
+{
+    PyObject_GC_UnTrack((PyObject *)self);
+    pygobject_weak_ref_clear(self);
+    PyObject_GC_Del(self);
+}
+
+static PyObject *
+pygobject_weak_ref_new(GObject *obj, PyObject *callback, PyObject *user_data)
+{
+    PyGObjectWeakRef *self;
+
+    self = PyObject_GC_New(PyGObjectWeakRef, &PyGObjectWeakRef_Type);
+    self->callback = callback;
+    self->user_data = user_data;
+    Py_XINCREF(self->callback);
+    Py_XINCREF(self->user_data);
+    self->obj = obj;
+    g_object_weak_ref(self->obj, (GWeakNotify) pygobject_weak_ref_notify, self);
+    if (callback != NULL) {
+          /* when we have a callback, we should INCREF the weakref
+           * object to make it stay alive even if it goes out of scope */
+        self->have_floating_ref = TRUE;
+        Py_INCREF((PyObject *) self);
+    }
+    return (PyObject *) self;
+}
+
+static PyObject *
+pygobject_weak_ref_unref(PyGObjectWeakRef *self, PyObject *args)
+{
+    if (!self->obj) {
+        PyErr_SetString(PyExc_ValueError, "weak ref already unreffed");
+        return NULL;
+    }
+    g_object_weak_unref(self->obj, (GWeakNotify) pygobject_weak_ref_notify, self);
+    self->obj = NULL;
+    if (self->have_floating_ref) {
+        self->have_floating_ref = FALSE;
+        Py_DECREF(self);
+    }
+    Py_INCREF(Py_None);
+    return Py_None;
+}
+
+static PyMethodDef pygobject_weak_ref_methods[] = {
+    { "unref", (PyCFunction)pygobject_weak_ref_unref, METH_NOARGS},
+    { NULL, NULL, 0}
+};
+
+static PyObject *
+pygobject_weak_ref_call(PyGObjectWeakRef *self, PyObject *args, PyObject *kw)
+{
+    static char *argnames[] = {NULL};
+
+    if (!PyArg_ParseTupleAndKeywords(args, kw, ":__call__", argnames))
+        return NULL;
+
+    if (self->obj)
+        return pygobject_new_full(self->obj, FALSE, NULL);
+    else {
+        Py_INCREF(Py_None);
+        return Py_None;
+    }
+}
+
+PyTypeObject PyGObjectWeakRef_Type = {
+    PyObject_HEAD_INIT(NULL)
+    0,                                          /* ob_size */
+    "gobject.GObjectWeakRef",                   /* tp_name */
+    sizeof(PyGObjectWeakRef),                   /* tp_basicsize */
+    0,                                          /* tp_itemsize */
+    (destructor)pygobject_weak_ref_dealloc,     /* tp_dealloc */
+    0,                                          /* tp_print */
+    0,                                          /* tp_getattr */
+    0,                                          /* tp_setattr */
+    0,                                          /* tp_compare */
+    0,                                          /* tp_repr */
+    0,                                          /* tp_as_number */
+    0, 						/* tp_as_sequence */
+    0,                                          /* tp_as_mapping */
+    0,                                          /* tp_hash */
+    (ternaryfunc)pygobject_weak_ref_call,  	/*tp_call*/
+    0,                                          /* tp_str */
+    (getattrofunc)0,            		/* tp_getattro */
+    (setattrofunc)0,            		/* tp_setattro */
+    0,                                          /* tp_as_buffer */
+    Py_TPFLAGS_DEFAULT|Py_TPFLAGS_HAVE_GC,	/* tp_flags */
+    "A GObject weak reference",                 /* tp_doc */
+    (traverseproc)pygobject_weak_ref_traverse,  /* tp_traverse */
+    (inquiry)pygobject_weak_ref_clear,          /* tp_clear */
+    0,                                          /* tp_richcompare */
+    0,                                          /* tp_weaklistoffset */
+    0,      					/* tp_iter */
+    0,                                          /* tp_iternext */
+    pygobject_weak_ref_methods,			/* tp_methods */
+    0,						/* tp_members */
+    0,						/* tp_getset */
+    NULL,					/* tp_base */
+    0,						/* tp_dict */
+    0,						/* tp_descr_get */
+    0,						/* tp_descr_set */
+    0,						/* tp_dictoffset */
+    0,						/* tp_init */
+    0,						/* tp_alloc */
+    0,						/* tp_new */
+    0,						/* tp_free */
+    0,						/* tp_is_gc */
+    NULL,					/* tp_bases */
+};
diff --git a/gobject/pygobject.h b/gobject/pygobject.h
index 68df6d3..71c7853 100644
--- a/gobject/pygobject.h
+++ b/gobject/pygobject.h
@@ -15,6 +15,8 @@ G_BEGIN_DECLS
   /* PyGClosure is a _private_ structure */
 typedef void (* PyClosureExceptionHandler) (GValue *ret, guint n_param_values, const GValue *params);
 typedef struct _PyGClosure PyGClosure;
+typedef struct _PyGObjectData PyGObjectData;
+
 struct _PyGClosure {
     GClosure closure;
     PyObject *callback;
@@ -23,12 +25,14 @@ struct _PyGClosure {
     PyClosureExceptionHandler exception_handler;
 };
 
+  /* closures is just an alias for what is found in the
+   * PyGObjectData */
 typedef struct {
     PyObject_HEAD
     GObject *obj;
     PyObject *inst_dict; /* the instance dictionary -- must be last */
     PyObject *weakreflist; /* list of weak references */
-    GSList *closures;
+    GSList *closures;           /* stale field; no longer updated DO-NOT-USE! */
 } PyGObject;
 
 #define pygobject_get(v) (((PyGObject *)(v))->obj)
diff --git a/gobject/pygtype.c b/gobject/pygtype.c
index 5bc29b5..0015b42 100644
--- a/gobject/pygtype.c
+++ b/gobject/pygtype.c
@@ -1328,14 +1328,18 @@ GClosure *
 gclosure_from_pyfunc(PyGObject *object, PyObject *func)
 {
     GSList *l;
-
-    for (l = object->closures; l; l = l->next) {
-	PyGClosure *pyclosure = l->data;
-	int res;
-	PyObject_Cmp(pyclosure->callback, func, &res);
-	if (!res) {
-	    return (GClosure*)pyclosure;
-	}
+    PyGObjectData *inst_data;
+    inst_data = pyg_object_peek_inst_data(object->obj);
+    if (inst_data)
+    {
+        for (l = inst_data->closures; l; l = l->next) {
+            PyGClosure *pyclosure = l->data;
+            int res;
+            PyObject_Cmp(pyclosure->callback, func, &res);
+            if (!res) {
+                return (GClosure*)pyclosure;
+            }
+        }
     }
     return NULL;
 }
diff --git a/tests/test_subtype.py b/tests/test_subtype.py
index 8aad307..244fdc4 100644
--- a/tests/test_subtype.py
+++ b/tests/test_subtype.py
@@ -1,8 +1,23 @@
 import unittest
+import weakref
+import gc
+import sys
 
 import testmodule
 from common import gobject, testhelper
-from gobject import GObject, GInterface
+from gobject import GObject, GInterface, GObjectMeta
+import gtk
+
+class _ClassInittableMetaType(GObjectMeta):
+    def __init__(cls, name, bases, namespace):
+        super(_ClassInittableMetaType, cls).__init__(name, bases, namespace)
+        cls.__class_init__(namespace)
+
+class ClassInittableObject(object):
+    __metaclass__ = _ClassInittableMetaType
+    def __class_init__(cls, namespace):
+        pass
+    __class_init__ = classmethod(__class_init__)
 
 class TestSubType(unittest.TestCase):
     def testSubType(self):
@@ -50,3 +65,172 @@ class TestSubType(unittest.TestCase):
         self.failUnless(isinstance(obj, Object2))
         self.assertEqual(obj.__gtype__.name, 'Object2')
 
+    def testUnregisteredSubclass(self):
+        class MyButton(gtk.Button):
+            def custom_method(self):
+                pass
+        b = MyButton()
+        self.assertEqual(type(b), MyButton)
+        box = gtk.EventBox()
+        box.add(b)
+        del b
+        b = box.child
+        self.assertEqual(type(b), MyButton)
+        try:
+            b.custom_method()
+        except AttributeError:
+            self.fail()
+
+    def testInstDict(self):
+        b = gtk.Button()
+        box = gtk.EventBox()
+        box.add(b)
+        b.xyz = "zbr"
+        del b
+        b = box.child
+        self.assert_(hasattr(b, "xyz"))
+        try:
+            xyz = b.xyz
+        except AttributeError:
+            self.fail()
+        self.assertEqual(xyz, "zbr")
+
+    def testImmediateCollection(self):
+        b = gtk.Button()
+        bref = weakref.ref(b)
+        while gc.collect():
+            pass
+        del b
+        self.assertEqual(gc.collect(), 0)
+        self.assertEqual(bref(), None)
+
+    def testGCCollection(self):
+        a = gtk.Button()
+        b = gtk.Button()
+        a.b = b
+        b.a = a
+        aref = weakref.ref(a)
+        bref = weakref.ref(b)
+        del a, b
+        while gc.collect():
+            pass
+        self.assertEqual(aref(), None)
+        self.assertEqual(bref(), None)
+
+    def testWrapperUnref(self):
+        b = gtk.Button()
+        bref = weakref.ref(b)
+        del b
+        self.assertEqual(bref(), None)
+
+    def testGObjectUnref(self):
+        b = gtk.Button()
+        bref = b.weak_ref()
+        self.assert_(bref() is b)
+        del b
+        self.assertEqual(bref(), None)
+
+    def testGCCollection(self):
+        a = gtk.Button()
+        b = gtk.Button()
+        a.b = b
+        b.a = a
+        aref = a.weak_ref()
+        bref = b.weak_ref()
+        del a, b
+        while gc.collect():
+            pass
+        self.assertEqual(aref(), None)
+        self.assertEqual(bref(), None)
+
+    def testGhostTwice(self):
+        b = gtk.Button()
+        bref = b.weak_ref()
+        box = gtk.EventBox()
+        box.add(b)
+        del b
+        b = box.child
+        del b
+        self.assertNotEqual(bref(), None)
+        box.destroy()
+        del box
+        self.assertEqual(bref(), None)
+        
+    def testGhostWeakref(self):
+        b = gtk.Button()
+        bref = b.weak_ref()
+        box = gtk.EventBox()
+        box.add(b)
+        del b
+        b = bref()
+        b.hide()
+        del box
+        b.hide()
+        del b
+
+    def testWeakRefCallback(self):
+        def callback(a, b, c):
+            self._wr_args = a, b, c
+        self._wr_args = None
+        b = gtk.Button()
+        bref = b.weak_ref(callback, 1, 2, 3)
+        del b
+        self.assertEqual(self._wr_args, (1, 2, 3))
+
+
+    def testCycle(self):
+
+        class _TestCycle(gtk.EventBox):
+            def __init__(self):
+                gtk.EventBox.__init__(self)
+                self.connect('notify', self.cb)
+
+                class DetectDel:
+                    def __del__(self):
+                        pass
+                        #print 'del'
+
+                self.d = DetectDel()
+
+            def cb(self, *args):
+                pass
+
+        a = _TestCycle()
+        a_d_id = id(a.d)
+        a.foo = "hello"
+        b = gtk.EventBox()
+        b.add(a)
+        #print "__dict__1: refcount=%i id=%i" % (sys.getrefcount(a.__dict__), id(a.__dict__))
+
+        del a
+        while gc.collect():
+            pass
+        a = b.child
+        #print "__dict__2: refcount=%i id=%i" % (sys.getrefcount(a.__dict__), id(a.__dict__))
+
+        del a
+        while gc.collect():
+            pass
+        a = b.child
+        #print "__dict__3: refcount=%i id=%i" % (sys.getrefcount(a.__dict__), id(a.__dict__))
+        
+        self.assert_(hasattr(a, 'd'))
+        self.assert_(hasattr(a, 'foo'))
+        self.assertEqual(a.foo, "hello")
+        self.assertEqual(id(a.d), a_d_id)
+
+    def testSimpleDecref(self):
+        class CallInDel:
+            def __init__(self, callback):
+                self.callback = callback
+                
+            def __del__(self):
+                if callable(self.callback):
+                    self.callback()
+        disposed_calls = []
+        def on_dispose():
+            disposed_calls.append(None)
+        gobj = GObject()
+        gobj.set_data('tmp', CallInDel(on_dispose))
+        del gobj
+        assert len(disposed_calls) == 1