Commit f0a0b6c2 authored by Steve Frécinaux's avatar Steve Frécinaux Committed by Tomeu Vizoso

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
parent cae2cf3d
......@@ -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;
......
......@@ -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);
......
......@@ -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;
}
......@@ -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);
......@@ -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)
......@@ -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
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