Re-implement wrapping of gst_pad_add_*probe in order to avoid leaks of user-data...

Original commit message from CVS:
reviewed by: Edward Hervey  <edward.hervey@collabora.co.uk>
* gst/gstpad.override:
* testsuite/test_pad.py:
Re-implement wrapping of gst_pad_add_*probe in order to avoid leaks of
user-data associated with the probes.
Fixes #504786

diff --git a/ChangeLog b/ChangeLog
index d096444..5670ec5 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2008-01-13  Olivier Crete  <tester@tester.ca>
+
+       reviewed by: Edward Hervey  <edward.hervey@collabora.co.uk>
+
+       * gst/gstpad.override:
+       * testsuite/test_pad.py:
+       Re-implement wrapping of gst_pad_add_*probe in order to avoid leaks of
+       user-data associated with the probes.
+       Fixes #504786
+
 2008-01-13  Edward Hervey  <edward.hervey@collabora.co.uk>
 
        * gst/pbutils.override:

diff --git a/gst/gstpad.override b/gst/gstpad.override
index 0db1789..70f76de 100644 (file)
--- a/gst/gstpad.override
+++ b/gst/gstpad.override
@@ -116,57 +116,6 @@ py_pad_private(PyGObject *pad)
     return pad_private ((GstPad *)pygobject_get(pad));
 }
 
-static gboolean
-probe_handler_marshal(GstPad *pad, GstMiniObject *data, gpointer user_data)
-{
-    PyGILState_STATE state;
-    PyObject *callback, *args;
-    PyObject *ret;
-    PyObject *py_data;
-    PyObject *py_user_data;
-    PyObject *repr;
-    gboolean res;
-    gint len, i;
-
-    g_return_val_if_fail(user_data != NULL, FALSE);
-
-    GST_LOG_OBJECT (pad, "marshalling probe handler for object %"
-        GST_PTR_FORMAT, data);
-    state = pyg_gil_state_ensure();
-
-    py_user_data = (PyObject *) user_data;
-
-    py_data = pygstminiobject_new(data);
-
-    callback = PyTuple_GetItem(py_user_data, 0);
-    args = Py_BuildValue("(NN)",
-             pygobject_new(G_OBJECT(pad)),
-             py_data);
-
-    len = PyTuple_Size(py_user_data);
-    for (i = 1; i < len; ++i) {
-        PyObject *tuple = args;
-        args = PySequence_Concat(tuple, PyTuple_GetItem(py_user_data, i));
-        Py_DECREF(tuple);
-    }
-    repr = PyObject_Repr (callback);
-    GST_LOG_OBJECT (pad, "calling callback %s", PyString_AsString (repr));
-    Py_DECREF (repr);
-
-    ret = PyObject_CallObject(callback, args);
-
-    if (!ret) {
-        PyErr_Print();
-        res = TRUE;
-    } else {
-        res = PyObject_IsTrue(ret);
-        Py_DECREF(ret);
-    }
-    Py_DECREF(args);
-    pyg_gil_state_release(state);
-
-    return res;
-}
 %%
 ignore
   gst_pad_select
@@ -895,90 +844,164 @@ override gst_pad_add_data_probe args
 static PyObject *
 _wrap_gst_pad_add_data_probe(PyGObject *self, PyObject *args)
 {
-    PyObject *callback, *cbargs = NULL, *data;
-    gulong sigid;
-    gint len;
+    GstPad *pad = GST_PAD(self->obj);
+    PyObject *method = NULL;
+    PyObject *rv = NULL;
+    PyObject *mylist = PyList_New(1);
+    PyObject *mynewlist = NULL;
+    PyObject *myargs = NULL;
+    PyObject *signalname = NULL;
+
+    signalname = PyString_FromString("have-data");
+
+    if (PyList_SetItem(mylist, 0, signalname)) {
+       Py_DECREF(mylist);
+       return NULL;
+    }
 
-    len = PyTuple_Size(args);
+    mynewlist = PySequence_InPlaceConcat(mylist, args);
+
+    Py_DECREF(mylist);
+
+    if (!mynewlist)
+       return NULL;
+
+    myargs = PyList_AsTuple(mynewlist);
+
+    Py_DECREF(mynewlist);
+
+    if (!myargs)
+       return NULL;
 
-    if (len < 1) {
-        PyErr_SetString(PyExc_TypeError, "Probe requires at least 1 arg");
+    method = PyObject_GetAttrString((PyObject*)self, "connect");
+
+    if (!method) {
+       Py_DECREF(mylist);
         return NULL;
     }
-    callback = PySequence_GetItem(args, 0);
-    if (!PyCallable_Check(callback)) {
-        PyErr_SetString(PyExc_TypeError, "callback is not callable");
-        return NULL;
+
+    GST_OBJECT_LOCK (pad);
+
+    rv = PyObject_CallObject(method, myargs);
+    if (rv) {
+       GST_PAD_DO_BUFFER_SIGNALS (pad)++;
+       GST_PAD_DO_EVENT_SIGNALS (pad)++;
     }
-    cbargs = PySequence_GetSlice(args, 1, len);
-    if (cbargs == NULL)
-        return NULL;
-    data = Py_BuildValue("(ON)", callback, cbargs);
-    if (data == NULL)
-        return NULL;
-    sigid = gst_pad_add_data_probe (GST_PAD (self->obj), (GCallback) probe_handler_marshal, data);
 
-    return PyLong_FromUnsignedLong(sigid);
+    GST_OBJECT_UNLOCK (pad);
+
+    Py_DECREF(myargs);
+    Py_DECREF(method);
+
+    return rv;
 }
 %%
 override gst_pad_add_event_probe args
 static PyObject *
 _wrap_gst_pad_add_event_probe(PyGObject *self, PyObject *args)
 {
-    PyObject *callback, *cbargs = NULL, *data;
-    gulong sigid;
-    gint len;
+    GstPad *pad = GST_PAD(self->obj);
+    PyObject *method = NULL;
+    PyObject *rv = NULL;
+    PyObject *mylist = PyList_New(1);
+    PyObject *mynewlist = NULL;
+    PyObject *myargs = NULL;
+    PyObject *signalname = NULL;
+
+    signalname = PyString_FromString("have-data::event");
+
+    if (PyList_SetItem(mylist, 0, signalname)) {
+       Py_DECREF(mylist);
+       return NULL;
+    }
 
-    len = PyTuple_Size(args);
+    mynewlist = PySequence_InPlaceConcat(mylist, args);
 
-    if (len < 1) {
-        PyErr_SetString(PyExc_TypeError, "Probe requires at least 1 arg");
-        return NULL;
-    }
-    callback = PySequence_GetItem(args, 0);
-    if (!PyCallable_Check(callback)) {
-        PyErr_SetString(PyExc_TypeError, "callback is not callable");
+    Py_DECREF(mylist);
+
+    if (!mynewlist)
+       return NULL;
+
+    myargs = PyList_AsTuple(mynewlist);
+
+    Py_DECREF(mynewlist);
+
+    if (!myargs)
+       return NULL;
+
+    method = PyObject_GetAttrString((PyObject*)self, "connect");
+
+    if (!method) {
+       Py_DECREF(mylist);
         return NULL;
     }
-    cbargs = PySequence_GetSlice(args, 1, len);
-    if (cbargs == NULL)
-        return NULL;
-    data = Py_BuildValue("(ON)", callback, cbargs);
-    if (data == NULL)
-        return NULL;
-    sigid = gst_pad_add_event_probe (GST_PAD (self->obj), (GCallback) probe_handler_marshal, data);
 
-    return PyLong_FromUnsignedLong(sigid);
+    GST_OBJECT_LOCK (pad);
+
+    rv = PyObject_CallObject(method, myargs);
+    if (rv)
+       GST_PAD_DO_EVENT_SIGNALS (pad)++;
+
+    GST_OBJECT_UNLOCK (pad);
+
+    Py_DECREF(myargs);
+    Py_DECREF(method);
+
+    return rv;
 }
 %%
 override gst_pad_add_buffer_probe args
 static PyObject *
 _wrap_gst_pad_add_buffer_probe(PyGObject *self, PyObject *args)
 {
-    PyObject *callback, *cbargs = NULL, *data;
-    gulong sigid;
-    gint len;
+    GstPad *pad = GST_PAD(self->obj);
+    PyObject *method = NULL;
+    PyObject *rv = NULL;
+    PyObject *mylist = PyList_New(1);
+    PyObject *mynewlist = NULL;
+    PyObject *myargs = NULL;
+    PyObject *signalname = NULL;
+
+    signalname = PyString_FromString("have-data::buffer");
+
+    if (PyList_SetItem(mylist, 0, signalname)) {
+       Py_DECREF(mylist);
+       return NULL;
+    }
 
-    len = PyTuple_Size(args);
+    mynewlist = PySequence_InPlaceConcat(mylist, args);
 
-    if (len < 1) {
-        PyErr_SetString(PyExc_TypeError, "Probe requires at least 1 arg");
-        return NULL;
-    }
-    callback = PySequence_GetItem(args, 0);
-    if (!PyCallable_Check(callback)) {
-        PyErr_SetString(PyExc_TypeError, "callback is not callable");
+    Py_DECREF(mylist);
+
+    if (!mynewlist)
+       return NULL;
+
+    myargs = PyList_AsTuple(mynewlist);
+
+    Py_DECREF(mynewlist);
+
+    if (!myargs)
+       return NULL;
+
+    method = PyObject_GetAttrString((PyObject*)self, "connect");
+
+    if (!method) {
+       Py_DECREF(mylist);
         return NULL;
     }
-    cbargs = PySequence_GetSlice(args, 1, len);
-    if (cbargs == NULL)
-        return NULL;
-    data = Py_BuildValue("(ON)", callback, cbargs);
-    if (data == NULL)
-        return NULL;
-    sigid = gst_pad_add_buffer_probe (GST_PAD (self->obj), (GCallback) probe_handler_marshal, data);
 
-    return PyLong_FromUnsignedLong(sigid);
+    GST_OBJECT_LOCK (pad);
+
+    rv = PyObject_CallObject(method, myargs);
+    if (rv)
+       GST_PAD_DO_BUFFER_SIGNALS (pad)++;
+
+    GST_OBJECT_UNLOCK (pad);
+
+    Py_DECREF(myargs);
+    Py_DECREF(method);
+
+    return rv;
 }
 %%
 override-slot GstPadTemplate.tp_getattr

diff --git a/testsuite/test_pad.py b/testsuite/test_pad.py
index 2a19e7d..a5b3a1c 100644 (file)
--- a/testsuite/test_pad.py
+++ b/testsuite/test_pad.py
@@ -457,18 +457,21 @@ class PadProbePipeTest(TestCase):
 
         handle = None
         self._num_times_called = 0
-        def buffer_probe(pad, buffer):
+        def buffer_probe(pad, buffer, data):
             self._num_times_called += 1
             pad.remove_buffer_probe(handle)
             return True
 
         pad = self.fakesrc.get_pad('src')
-        handle = pad.add_buffer_probe(buffer_probe)
+        data = []
+        handle = pad.add_buffer_probe(buffer_probe, data)
         self.pipeline.set_state(gst.STATE_PLAYING)
         m = self.pipeline.get_bus().poll(gst.MESSAGE_EOS, -1)
         assert m
         assert self._num_times_called == 1
         self.pipeline.set_state(gst.STATE_NULL)
+        assert sys.getrefcount(buffer_probe) == 2
+        assert sys.getrefcount(data) == 2
         # FIXME: having m going out of scope doesn't seem to be enough
         # to get it gc collected, and it keeps a ref to the pipeline.
         # Look for a way to not have to do this explicitly