[patch][python] Audit tp_dealloc callbacks to make sure they preserve exception state

Simon McVittie simon.mcvittie at collabora.co.uk
Wed Feb 7 06:27:43 PST 2007


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

While fixing memory leaks I noticed that tp_dealloc isn't meant to alter
the exception state. Most of dbus-python's tp_dealloc implementations
are trivial, but Connection does things which might trigger callbacks,
so we should use PyErr_Fetch and PyErr_Restore.

Also, the MainLoop tp_dealloc invokes the "free" callback, so that
callback has the same requirement if it might invoke Python API.

	Simon

- From 278b57d9f4a1aed4f0296b17a94bde2a36145a45 Mon Sep 17 00:00:00 2001
From: Simon McVittie <simon.mcvittie at collabora.co.uk>
Date: Wed, 7 Feb 2007 13:15:17 +0000
Subject: [PATCH] Audit tp_dealloc callbacks to make sure they preserve the exception state.
* Connection: use PyErr_Fetch and PyErr_Restore to preserve exception state
* MainLoop: add a comment indicating that the "free" callback needs to do the
  same if it might alter the exception state
- ---
 _dbus_bindings/conn.c     |    5 +++++
 _dbus_bindings/mainloop.c |    3 ++-
 2 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/_dbus_bindings/conn.c b/_dbus_bindings/conn.c
index 51ed5f1..23b5447 100644
- --- a/_dbus_bindings/conn.c
+++ b/_dbus_bindings/conn.c
@@ -328,9 +328,13 @@ Connection_tp_new(PyTypeObject *cls, PyObject *args, PyObject *kwargs)
 static void Connection_tp_dealloc(Connection *self)
 {
     DBusConnection *conn = self->conn;
+    PyObject *et, *ev, *etb;
     PyObject *filters = self->filters;
     PyObject *object_paths = self->object_paths;
 
+    /* avoid clobbering any pending exception */
+    PyErr_Fetch(&et, &ev, &etb);
+
     if (self->weaklist) {
         PyObject_ClearWeakRefs((PyObject *)self);
     }
@@ -367,6 +371,7 @@ static void Connection_tp_dealloc(Connection *self)
     }
 
     DBG("Connection at %p: freeing self", self);
+    PyErr_Restore(et, ev, etb);
     (self->ob_type->tp_free)((PyObject *)self);
 }
 
diff --git a/_dbus_bindings/mainloop.c b/_dbus_bindings/mainloop.c
index ed486ed..9d7de07 100644
- --- a/_dbus_bindings/mainloop.c
+++ b/_dbus_bindings/mainloop.c
@@ -381,7 +381,8 @@ typedef struct {
     /* Called with the GIL held, should set a Python exception on error */
     dbus_bool_t (*set_up_connection_cb)(DBusConnection *, void *);
     dbus_bool_t (*set_up_server_cb)(DBusServer *, void *);
- -    /* Called in a destructor. */
+    /* Called in a destructor. Must not touch the exception state (use
+     * PyErr_Fetch and PyErr_Restore if necessary). */
     void (*free_cb)(void *);
     void *data;
 } NativeMainLoop;
- -- 
1.4.4.4

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: OpenPGP key: http://www.pseudorandom.co.uk/2003/contact/ or pgp.net

iD8DBQFFyeHeWSc8zVUw7HYRApvyAKDKMzbPlIT7yF34khJru6WJsH5bbwCfcYWT
O9JCOUVhr1nueFdIAlGpavQ=
=YLWF
-----END PGP SIGNATURE-----


More information about the dbus mailing list