From dd6bf4a4d46da1d1bad89fb70b8f4cd43e655c03 Mon Sep 17 00:00:00 2001 From: Thomas Vander Stichele Date: Thu, 2 Apr 2009 17:21:58 +0200 Subject: [PATCH 1/2] Fix for #577735: do_handle_message leaks messages --- gst/gstbin.override | 80 ++++++++++++++++++++++++++++++++++++++ testsuite/test_pipeline.py | 47 +++++++++++++++++++++- 2 files changed, 126 insertions(+), 1 deletion(-) diff --git a/gst/gstbin.override b/gst/gstbin.override index a1a21b4678..c47316389e 100644 --- a/gst/gstbin.override +++ b/gst/gstbin.override @@ -183,3 +183,83 @@ _wrap_GstBin__do_handle_message(PyObject *cls, PyObject *args, PyObject *kwargs) Py_INCREF(Py_None); return Py_None; } + +%% +override GstBin__proxy_do_handle_message +static void +_wrap_GstBin__proxy_do_handle_message(GstBin *self, GstMessage*message) +{ + PyGILState_STATE __py_state; + PyObject *py_self; + PyObject *py_message = NULL; + PyObject *py_retval; + PyObject *py_args; + PyObject *py_method; + + __py_state = pyg_gil_state_ensure(); + py_self = pygobject_new((GObject *) self); + if (!py_self) { + if (PyErr_Occurred()) + PyErr_Print(); + pyg_gil_state_release(__py_state); + return; + } + if (message) { + py_message = pygstminiobject_new((GstMiniObject *) message); + gst_mini_object_unref ((GstMiniObject *) message); + } else { + Py_INCREF(Py_None); + py_message = Py_None; + } + + py_args = PyTuple_New(1); + Py_INCREF(py_message); + PyTuple_SET_ITEM(py_args, 0, py_message); + + py_method = PyObject_GetAttrString(py_self, "do_handle_message"); + if (!py_method) { + if (PyErr_Occurred()) + PyErr_Print(); + Py_DECREF(py_args); + gst_mini_object_ref ((GstMiniObject *) message); Py_DECREF(py_message); + Py_DECREF(py_self); + pyg_gil_state_release(__py_state); + return; + } + py_retval = PyObject_CallObject(py_method, py_args); + if (!py_retval) { + if (PyErr_Occurred()) + PyErr_Print(); + Py_DECREF(py_method); + Py_DECREF(py_args); + gst_mini_object_ref ((GstMiniObject *) message); Py_DECREF(py_message); + Py_DECREF(py_self); + pyg_gil_state_release(__py_state); + return; + } + if (py_retval != Py_None) { + if (PyErr_Occurred()) + PyErr_Print(); + PyErr_SetString(PyExc_TypeError, "retval should be None"); + Py_DECREF(py_retval); + Py_DECREF(py_method); + Py_DECREF(py_args); + gst_mini_object_ref ((GstMiniObject *) message); Py_DECREF(py_message); + Py_DECREF(py_self); + pyg_gil_state_release(__py_state); + return; + } + + + Py_DECREF(py_retval); + Py_DECREF(py_method); + Py_DECREF(py_args); + gst_mini_object_ref ((GstMiniObject *) message); Py_DECREF(py_message); + + /* #577735: since the bus handler will return BUS_DROP, we should unref. + This is the only change from the generated code. */ + gst_mini_object_unref ((GstMiniObject *) message); + + Py_DECREF(py_self); + pyg_gil_state_release(__py_state); +} diff --git a/testsuite/test_pipeline.py b/testsuite/test_pipeline.py index ec6afae15f..77f84daa2d 100644 --- a/testsuite/test_pipeline.py +++ b/testsuite/test_pipeline.py @@ -163,6 +163,51 @@ class PipelineAndBus(TestCase): self.assertEquals(self.pipeline.get_state(), (gst.STATE_CHANGE_SUCCESS, gst.STATE_NULL, gst.STATE_VOID_PENDING)) self.gccollect() - + +class TestPipeSub(gst.Pipeline): + def do_handle_message(self, message): + self.debug('do_handle_message') + gst.Pipeline.do_handle_message(self, message) + self.message = True +gobject.type_register(TestPipeSub) + +class TestPipeSubSub(TestPipeSub): + def do_handle_message(self, message): + self.debug('do_handle_message') + TestPipeSub.do_handle_message(self, message) + self.message = True +gobject.type_register(TestPipeSubSub) + + +class TestSubClass(TestCase): + def setUp(self): + self.gctrack() + + def tearDown(self): + self.gccollect() + self.gcverify() + + def testSubClass(self): + p = TestPipeSub() + u = gst.element_factory_make('uridecodebin') + self.assertEquals(u.__grefcount__, 1) + # adding uridecodebin triggers a clock-provide message; + # this message should be dropped, and thus not affect + # the refcount of u beyond the parenting. + p.add(u) + self.failUnless(getattr(p, 'message')) + self.assertEquals(u.__grefcount__, 2) + del p + self.assertEquals(u.__grefcount__, 1) + + def testSubSubClass(self): + p = TestPipeSubSub() + u = gst.element_factory_make('uridecodebin') + self.assertEquals(u.__grefcount__, 1) + p.add(u) + self.assertEquals(u.__grefcount__, 2) + del p + self.assertEquals(u.__grefcount__, 1) + if __name__ == "__main__": unittest.main() From f99e67aa24878d26dbd7f494f310bb466c5bd1cd Mon Sep 17 00:00:00 2001 From: Thomas Vander Stichele Date: Thu, 2 Apr 2009 18:06:12 +0200 Subject: [PATCH 2/2] make sure that we actually get the clock-provide message --- testsuite/test_pipeline.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/testsuite/test_pipeline.py b/testsuite/test_pipeline.py index 77f84daa2d..a84b98843a 100644 --- a/testsuite/test_pipeline.py +++ b/testsuite/test_pipeline.py @@ -168,17 +168,17 @@ class TestPipeSub(gst.Pipeline): def do_handle_message(self, message): self.debug('do_handle_message') gst.Pipeline.do_handle_message(self, message) - self.message = True + self.type = message.type gobject.type_register(TestPipeSub) class TestPipeSubSub(TestPipeSub): def do_handle_message(self, message): self.debug('do_handle_message') TestPipeSub.do_handle_message(self, message) - self.message = True gobject.type_register(TestPipeSubSub) +# see http://bugzilla.gnome.org/show_bug.cgi?id=577735 class TestSubClass(TestCase): def setUp(self): self.gctrack() @@ -191,20 +191,25 @@ class TestSubClass(TestCase): p = TestPipeSub() u = gst.element_factory_make('uridecodebin') self.assertEquals(u.__grefcount__, 1) + self.failIf(getattr(p, 'type', None)) # adding uridecodebin triggers a clock-provide message; # this message should be dropped, and thus not affect # the refcount of u beyond the parenting. p.add(u) - self.failUnless(getattr(p, 'message')) + self.assertEquals(getattr(p, 'type', None), gst.MESSAGE_CLOCK_PROVIDE) self.assertEquals(u.__grefcount__, 2) del p self.assertEquals(u.__grefcount__, 1) def testSubSubClass(self): + # Edward is worried that a subclass of a subclass will screw up + # the refcounting wrt. GST_BUS_DROP p = TestPipeSubSub() u = gst.element_factory_make('uridecodebin') self.assertEquals(u.__grefcount__, 1) + self.failIf(getattr(p, 'type', None)) p.add(u) + self.assertEquals(getattr(p, 'type', None), gst.MESSAGE_CLOCK_PROVIDE) self.assertEquals(u.__grefcount__, 2) del p self.assertEquals(u.__grefcount__, 1)