[pygobject] Fix reference leaks for GInitiallyUnowned objects
- From: Tomeu Vizoso <tomeuv src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [pygobject] Fix reference leaks for GInitiallyUnowned objects
- Date: Thu, 20 Jan 2011 14:21:04 +0000 (UTC)
commit f0a0b6c2eda89622de2b1e5ebb6a48103ad72a42
Author: Steve Frécinaux <code istique net>
Date: Thu Jan 20 14:14:15 2011 +0100
Fix reference leaks for GInitiallyUnowned objects
References were leaked for GInitiallyUnowned objects which got their
wrappers created several times, because someone else holds reference
on it and it got out of python scope at some point.
https://bugzilla.gnome.org/show_bug.cgi?id=639949
gobject/gobjectmodule.c | 2 +
gobject/pygobject.c | 14 +++++-----
tests/test-floating.c | 36 ++++++++++++++++++++++++++
tests/test-floating.h | 21 +++++++++++++++
tests/test_gobject.py | 63 ++++++++++++++++++++++++++++++++++++++++++++++
tests/testhelpermodule.c | 50 ++++++++++++++++++++++++++++++++++++
6 files changed, 179 insertions(+), 7 deletions(-)
---
diff --git a/gobject/gobjectmodule.c b/gobject/gobjectmodule.c
index 5f180e3..fb095f7 100644
--- a/gobject/gobjectmodule.c
+++ b/gobject/gobjectmodule.c
@@ -1756,6 +1756,7 @@ pyg_object_new (PyGObject *self, PyObject *args, PyObject *kwargs)
g_type_class_unref(class);
if (obj) {
+ pygobject_sink (obj);
self = (PyGObject *) pygobject_new_full((GObject *)obj, FALSE, NULL);
g_object_unref(obj);
} else
@@ -2255,6 +2256,7 @@ pygobject_constructv(PyGObject *self,
pygobject_init_wrapper_set((PyObject *) self);
obj = g_object_newv(pyg_type_from_object((PyObject *) self),
n_parameters, parameters);
+ pygobject_sink (obj);
pygobject_init_wrapper_set(NULL);
if (self->obj == NULL) {
self->obj = obj;
diff --git a/gobject/pygobject.c b/gobject/pygobject.c
index eee3963..262cf69 100644
--- a/gobject/pygobject.c
+++ b/gobject/pygobject.c
@@ -146,12 +146,13 @@ pygobject_sink(GObject *obj)
}
}
}
- if (G_IS_INITIALLY_UNOWNED(obj) && !g_object_is_floating(obj)) {
- /* GtkWindow and GtkInvisible does not return a ref to caller of
- * g_object_new.
- */
- g_object_ref(obj);
- } else if (g_object_is_floating(obj)) {
+ /* The default behaviour for GInitiallyUnowned subclasses is to call ref_sink().
+ * - if the object is new and owned by someone else, its ref has been sunk and
+ * we need to keep the one from that someone and add our own "fresh ref"
+ * - if the object is not and owned by nobody, its ref is floating and we need
+ * to transform it into a regular ref.
+ */
+ if (G_IS_INITIALLY_UNOWNED(obj)) {
g_object_ref_sink(obj);
}
}
@@ -631,7 +632,6 @@ pygobject_register_wrapper(PyObject *self)
gself = (PyGObject *)self;
- pygobject_sink(gself->obj);
g_assert(gself->obj->ref_count >= 1);
/* save wrapper pointer so we can access it later */
g_object_set_qdata_full(gself->obj, pygobject_wrapper_key, gself, NULL);
diff --git a/tests/test-floating.c b/tests/test-floating.c
index 393784e..8e8ba5d 100644
--- a/tests/test-floating.c
+++ b/tests/test-floating.c
@@ -123,3 +123,39 @@ test_owned_by_library_get_instance_list (void)
{
return obl_instance_list;
}
+
+/* TestFloatingAndSunk
+ * This object is mimicking the GtkWindow behaviour, ie a GInitiallyUnowned subclass
+ * whose floating reference has already been sunk when g_object_new() returns it.
+ * The reference is already sunk because the instance is already owned by the instance
+ * list.
+ */
+
+G_DEFINE_TYPE(TestFloatingAndSunk, test_floating_and_sunk, G_TYPE_INITIALLY_UNOWNED)
+
+static GSList *fas_instance_list = NULL;
+
+static void
+test_floating_and_sunk_class_init (TestFloatingAndSunkClass *klass)
+{
+}
+
+static void
+test_floating_and_sunk_init (TestFloatingAndSunk *self)
+{
+ g_object_ref_sink (self);
+ fas_instance_list = g_slist_prepend (fas_instance_list, self);
+}
+
+void
+test_floating_and_sunk_release (TestFloatingAndSunk *self)
+{
+ fas_instance_list = g_slist_remove (fas_instance_list, self);
+ g_object_unref (self);
+}
+
+GSList *
+test_floating_and_sunk_get_instance_list (void)
+{
+ return fas_instance_list;
+}
diff --git a/tests/test-floating.h b/tests/test-floating.h
index 05cd394..bf4e101 100644
--- a/tests/test-floating.h
+++ b/tests/test-floating.h
@@ -78,3 +78,24 @@ typedef struct {
GType test_owned_by_library_get_type (void);
void test_owned_by_library_release (TestOwnedByLibrary *self);
GSList *test_owned_by_library_get_instance_list (void);
+
+/* TestFloatingAndSunk */
+
+typedef struct {
+ GInitiallyUnowned parent;
+} TestFloatingAndSunk;
+
+typedef struct {
+ GInitiallyUnownedClass parent_class;
+} TestFloatingAndSunkClass;
+
+#define TEST_TYPE_FLOATING_AND_SUNK (test_floating_and_sunk_get_type())
+#define TEST_FLOATING_AND_SUNK(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), TEST_TYPE_FLOATING_AND_SUNK, TestFloatingAndSunk))
+#define TEST_FLOATING_AND_SUNK_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST ((klass), TEST_TYPE_FLOATING_AND_SUNK, TestFloatingAndSunkClass))
+#define TEST_IS_FLOATING_AND_SUNK(obj) (G_TYPE_CHECK_INSTANCE_TYPE ((obj), TEST_TYPE_FLOATING_AND_SUNK))
+#define TEST_IS_FLOATING_AND_SUNK_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((obj), TEST_TYPE_FLOATING_AND_SUNK))
+#define TEST_FLOATING_AND_SUNK_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS((obj), TEST_TYPE_FLOATING_AND_SUNK, TestFloatingAndSunkClass))
+
+GType test_floating_and_sunk_get_type (void);
+void test_floating_and_sunk_release (TestFloatingAndSunk *self);
+GSList *test_floating_and_sunk_get_instance_list (void);
diff --git a/tests/test_gobject.py b/tests/test_gobject.py
index cfcebfd..9dfe166 100644
--- a/tests/test_gobject.py
+++ b/tests/test_gobject.py
@@ -96,3 +96,66 @@ class TestReferenceCounting(unittest.TestCase):
obj.release()
self.assertEquals(obj.__grefcount__, 1)
+
+ def testFloatingAndSunk(self):
+ # Upon creation, the refcount of the object should be 2:
+ # - someone already has a reference on the new object.
+ # - the python wrapper should hold its own reference.
+ obj = testhelper.FloatingAndSunk()
+ self.assertEquals(obj.__grefcount__, 2)
+
+ # We ask the library to release its reference, so the only
+ # remaining ref should be our wrapper's. Once the wrapper
+ # will run out of scope, the object will get finalized.
+ obj.release()
+ self.assertEquals(obj.__grefcount__, 1)
+
+ def testFloatingAndSunkOutOfScope(self):
+ obj = testhelper.FloatingAndSunk()
+ self.assertEquals(obj.__grefcount__, 2)
+
+ # We are manually taking the object out of scope. This means
+ # that our wrapper has been freed, and its reference dropped. We
+ # cannot check it but the refcount should now be 1 (the ref held
+ # by the library is still there, we didn't call release()
+ obj = None
+
+ # When we get the object back from the lib, the wrapper is
+ # re-created, so our refcount will be 2 once again.
+ obj = testhelper.floating_and_sunk_get_instance_list()[0]
+ self.assertEquals(obj.__grefcount__, 2)
+
+ obj.release()
+ self.assertEquals(obj.__grefcount__, 1)
+
+
+ def testFloatingAndSunkUsingGObjectNew(self):
+ # Upon creation, the refcount of the object should be 2:
+ # - someone already has a reference on the new object.
+ # - the python wrapper should hold its own reference.
+ obj = gobject.new(testhelper.FloatingAndSunk)
+ self.assertEquals(obj.__grefcount__, 2)
+
+ # We ask the library to release its reference, so the only
+ # remaining ref should be our wrapper's. Once the wrapper
+ # will run out of scope, the object will get finalized.
+ obj.release()
+ self.assertEquals(obj.__grefcount__, 1)
+
+ def testFloatingAndSunkOutOfScopeUsingGObjectNew(self):
+ obj = gobject.new(testhelper.FloatingAndSunk)
+ self.assertEquals(obj.__grefcount__, 2)
+
+ # We are manually taking the object out of scope. This means
+ # that our wrapper has been freed, and its reference dropped. We
+ # cannot check it but the refcount should now be 1 (the ref held
+ # by the library is still there, we didn't call release()
+ obj = None
+
+ # When we get the object back from the lib, the wrapper is
+ # re-created, so our refcount will be 2 once again.
+ obj = testhelper.floating_and_sunk_get_instance_list()[0]
+ self.assertEquals(obj.__grefcount__, 2)
+
+ obj.release()
+ self.assertEquals(obj.__grefcount__, 1)
diff --git a/tests/testhelpermodule.c b/tests/testhelpermodule.c
index 9a0be36..2fa7017 100644
--- a/tests/testhelpermodule.c
+++ b/tests/testhelpermodule.c
@@ -245,6 +245,21 @@ static const PyMethodDef _PyTestOwnedByLibrary_methods[] = {
{ NULL, NULL, 0, NULL }
};
+/* TestFloatingAndSunk */
+PYGLIB_DEFINE_TYPE("testhelper.FloatingAndSunk", PyTestFloatingAndSunk_Type, PyGObject);
+
+static PyObject *
+_wrap_test_floating_and_sunk_release (PyGObject *self)
+{
+ test_floating_and_sunk_release (TEST_FLOATING_AND_SUNK (self->obj));
+ return Py_None;
+}
+
+static const PyMethodDef _PyTestFloatingAndSunk_methods[] = {
+ { "release", (PyCFunction)_wrap_test_floating_and_sunk_release, METH_NOARGS, NULL },
+ { NULL, NULL, 0, NULL }
+};
+
#include <string.h>
#include <glib-object.h>
@@ -470,6 +485,29 @@ _wrap_test_owned_by_library_get_instance_list (PyObject *self)
return py_list;
}
+static PyObject *
+_wrap_test_floating_and_sunk_get_instance_list (PyObject *self)
+{
+ PyObject *py_list, *py_obj;
+ GSList *list, *tmp;
+
+ list = test_floating_and_sunk_get_instance_list ();
+
+ if ((py_list = PyList_New (0)) == NULL) {
+ return NULL;
+ }
+ for (tmp = list; tmp != NULL; tmp = tmp->next) {
+ py_obj = pygobject_new (G_OBJECT (tmp->data));
+ if (py_obj == NULL) {
+ Py_DECREF (py_list);
+ return NULL;
+ }
+ PyList_Append (py_list, py_obj);
+ Py_DECREF (py_obj);
+ }
+ return py_list;
+}
+
static PyMethodDef testhelper_functions[] = {
{ "get_test_thread", (PyCFunction)_wrap_get_test_thread, METH_NOARGS },
{ "get_unknown", (PyCFunction)_wrap_get_unknown, METH_NOARGS },
@@ -480,6 +518,7 @@ static PyMethodDef testhelper_functions[] = {
{ "test_value_array", (PyCFunction)_wrap_test_value_array, METH_VARARGS },
{ "test_gerror_exception", (PyCFunction)_wrap_test_gerror_exception, METH_VARARGS },
{ "owned_by_library_get_instance_list", (PyCFunction)_wrap_test_owned_by_library_get_instance_list, METH_NOARGS },
+ { "floating_and_sunk_get_instance_list", (PyCFunction)_wrap_test_floating_and_sunk_get_instance_list, METH_NOARGS },
{ NULL, NULL }
};
@@ -558,6 +597,17 @@ PYGLIB_MODULE_START(testhelper, "testhelper")
Py_BuildValue("(O)",
&PyGObject_Type));
pyg_set_object_has_new_constructor(TEST_TYPE_OWNED_BY_LIBRARY);
+
+ /* TestFloatingAndSunk */
+ PyTestFloatingAndSunk_Type.tp_flags = (Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE);
+ PyTestFloatingAndSunk_Type.tp_methods = (struct PyMethodDef*)_PyTestFloatingAndSunk_methods;
+ PyTestFloatingAndSunk_Type.tp_weaklistoffset = offsetof(PyGObject, weakreflist);
+ PyTestFloatingAndSunk_Type.tp_dictoffset = offsetof(PyGObject, inst_dict);
+ pygobject_register_class(d, "FloatingAndSunk", TEST_TYPE_FLOATING_AND_SUNK,
+ &PyTestFloatingAndSunk_Type,
+ Py_BuildValue("(O)",
+ &PyGObject_Type));
+ pyg_set_object_has_new_constructor(TEST_TYPE_FLOATING_AND_SUNK);
}
PYGLIB_MODULE_END
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
