GNOME Bugzilla – Bug 660357
pad: fix Py_DECREF of null pointer in pad probe and pad block marshallers
Last modified: 2012-03-07 16:54:11 UTC
Created attachment 197661 [details] [review] Fix decref of null pointer I caught a segfault in gst-python that I can't reproduce, but I have a coredump. It's seems that if the call to the callback fails, args can be NULL. Attached patch. (gdb) frame 0
+ Trace 228625
1381 callback = PyTuple_GetItem(py_user_data, 0); 1382 args = Py_BuildValue("(NO)", 1383 pygobject_new(G_OBJECT(pad)), 1384 blocked ? Py_True : Py_False); 1385 1386 { 1387 PyObject *tmp = args; 1388 args = PySequence_Concat(tmp, PyTuple_GetItem(py_user_data, 1)); 1389 Py_DECREF (tmp); 1390 } 1391 1392 ret = PyObject_CallObject(callback, args); 1393 Py_DECREF(args); 1394 1395 if (!ret) 1396 PyErr_Print(); 1397 else 1398 Py_DECREF(ret); 1399 1400 pyg_gil_state_release(state); 1401 } 1402 1403 static PyObject * 1404 _wrap_gst_pad_set_blocked_async (PyGObject *self, PyObject *args) 1405 { (gdb) print *args Cannot access memory at address 0x0 (gdb) print *ret Cannot access memory at address 0x0 (gdb)
Comment on attachment 197661 [details] [review] Fix decref of null pointer >@@ -1395,7 +1395,8 @@ pad_block_callback_marshal(GstPad *pad, gboolean blocked, gpointer user_data) > } > > ret = PyObject_CallObject(callback, args); >- Py_DECREF(args); >+ if (args != NULL) >+ Py_DECREF(args); > > if (!ret) > PyErr_Print(); It obviously "can't hurt", but I wonder this: if args is NULL there, doesn't that mean that PySequence_Concat has failed? (Assuming PyObject_CallObject is not some kind of weird macro that can mess with 'args') and if PySequence_Concat failed for some reason, should we still be doing the PyObject_CallObject in the first place with NULL args instead of erroring out?
Andoni, can you please respond to comment#1 ?
I have seen this bug reproduced several times before applying the patch although Tim might be right and probably the callback shouldn't even be called if args is NULL. Navigating through CPython I see that it's a common practice checking if args is NULL before calling CallObject().
Created attachment 203255 [details] [review] Fix decref of null pointer
Tim-Philipp, updated patch is available.
Thanks for the updated patch, but I'd contend that this patch hasn't been tested at all, given that it even made 'make check' fail ;-) commit 9f163262f02a0a33544ff6941e58b983184220c8 Author: Tim-Philipp Müller <tim.muller@collabora.co.uk> Date: Wed Mar 7 16:50:11 2012 +0000 pad: fix unit test again after previous commit https://bugzilla.gnome.org/show_bug.cgi?id=660357 commit c3dab3df761a7ae0680753b4993cefcff56a0482 Author: Andoni Morales Alastruey <amorales@flumotion.com> Date: Wed Sep 28 15:16:07 2011 +0200 pad: fix Py_DECREF of null pointer in pad probe and pad block marshallers https://bugzilla.gnome.org/show_bug.cgi?id=660357