[patch] python-dbus 0.80.1 memory leak issue

Simon McVittie simon.mcvittie at collabora.co.uk
Mon Feb 5 08:30:04 PST 2007


On Wed, 31 Jan 2007 at 19:55:03 +0000, Simon McVittie wrote:
> On Wed, 31 Jan 2007 at 11:58:51 -0500, John (J5) Palmieri wrote:
> > The first goto will call Py_XDECREF(handler) on a NULL.  Does Py_XDECREF
> > handle NULL's as noops?
> 
> Yes, that's what the X means. Py_DECREF is the fast unchecked version.

Some more leak fixes attached. There's at least one more - if you allocate
dbus.String objects in a fast loop, memory usage increases over time -
but I can't see what could possibly be causing it! Perhaps someone else
could have a look at the String constructor in _dbus_bindings/string.c and
see if there's something I haven't spotted? The following script reproduces it:

#!/usr/bin/python
import sys

import _dbus_bindings

while True:

    x = _dbus_bindings.String('12345rgehruhgeiruhgeiurhgeliurhgeuirh')
    print id(x),
    print sys.getrefcount(x)
    del x

The String always seems to be created and destroyed at the same address,
which is good, but perhaps its memory buffer isn't being freed correctly or
something. I'm just delegating to the unicode destructor to do that though,
so it really ought to work.

	Simon
-------------- next part --------------
>From 95c0d3618041e8c8f9173a3eb8e8ddc93c456952 Mon Sep 17 00:00:00 2001
From: Simon McVittie <simon.mcvittie at collabora.co.uk>
Date: Mon, 5 Feb 2007 13:17:12 +0000
Subject: [PATCH] Fix a couple of memory leaks - D-Bus signature strings, and decoded Unicode objects

---
 _dbus_bindings/message-get-args.c |   18 +++++++++++-------
 1 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/_dbus_bindings/message-get-args.c b/_dbus_bindings/message-get-args.c
index dd8d425..cf5a016 100644
--- a/_dbus_bindings/message-get-args.c
+++ b/_dbus_bindings/message-get-args.c
@@ -118,9 +118,17 @@ _message_iter_get_dict(DBusMessageIter *iter,
     PyObject *ret;
     int status;
 
+    if (!sig_str) {
+        PyErr_NoMemory();
+        return NULL;
+    }
     sig = PyObject_CallFunction((PyObject *)&DBusPySignature_Type,
                                 "(s#)", sig_str+2,
                                 (Py_ssize_t)strlen(sig_str)-3);
+    dbus_free(sig_str);
+    if (!sig) {
+        return NULL;
+    }
     status = PyDict_SetItem(kwargs, dbus_py_signature_const, sig);
     Py_DECREF(sig);
     if (status < 0) {
@@ -234,7 +242,7 @@ _message_iter_get_pyobject(DBusMessageIter *iter,
                                     args, kwargs);
             }
             else {
-                args = Py_BuildValue("(O)", PyUnicode_DecodeUTF8(u.s,
+                args = Py_BuildValue("(N)", PyUnicode_DecodeUTF8(u.s,
                                                                  strlen(u.s),
                                                                  NULL));
                 if (!args) {
@@ -453,12 +461,8 @@ _message_iter_get_pyobject(DBusMessageIter *iter,
                          "message", type);
     }
 
-    if (args) {
-        Py_DECREF(args);
-    }
-    if (kwargs) {
-        Py_DECREF(kwargs);
-    }
+    Py_XDECREF(args);
+    Py_XDECREF(kwargs);
     return ret;
 }
 
-- 
1.4.4.4

-------------- next part --------------
>From 630f912b2155e6328a3fa48deb832f5c3d114b94 Mon Sep 17 00:00:00 2001
From: Simon McVittie <simon.mcvittie at collabora.co.uk>
Date: Mon, 5 Feb 2007 13:18:19 +0000
Subject: [PATCH] Switch _IntBase back to using generic alloc/free implementation rather than half-participating in the int free list (which would result in _IntBase instances leaking)

---
 _dbus_bindings/abstract.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/_dbus_bindings/abstract.c b/_dbus_bindings/abstract.c
index 478ac86..8e9507e 100644
--- a/_dbus_bindings/abstract.c
+++ b/_dbus_bindings/abstract.c
@@ -132,8 +132,9 @@ PyTypeObject DBusPyIntBase_Type = {
     0,                                      /* tp_descr_set */
     0,                                      /* tp_dictoffset */
     0,                                      /* tp_init */
-    0,                                      /* tp_alloc */
+    PyType_GenericAlloc,                    /* tp_alloc */
     DBusPythonInt_tp_new,                   /* tp_new */
+    PyObject_Del,                           /* tp_free */
 };
 
 /* Support code for float subclasses. ================================ */
-- 
1.4.4.4

-------------- next part --------------
>From 192bd48b8a17e4f62400b64e037df22c3b47de88 Mon Sep 17 00:00:00 2001
From: Simon McVittie <simon.mcvittie at collabora.co.uk>
Date: Mon, 5 Feb 2007 15:18:14 +0000
Subject: [PATCH] Don't leak memory in _StringBase and _LongBase repr()

---
 _dbus_bindings/abstract.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/_dbus_bindings/abstract.c b/_dbus_bindings/abstract.c
index 8e9507e..57a4e04 100644
--- a/_dbus_bindings/abstract.c
+++ b/_dbus_bindings/abstract.c
@@ -297,8 +297,12 @@ DBusPythonString_tp_repr(PyObject *self)
 
     if (!parent_repr) return NULL;
     vl_obj = PyObject_GetAttr(self, dbus_py_variant_level_const);
-    if (!vl_obj) return NULL;
+    if (!vl_obj) {
+        Py_DECREF(parent_repr);
+        return NULL;
+    }
     variant_level = PyInt_AsLong(vl_obj);
+    Py_DECREF(vl_obj);
     if (variant_level > 0) {
         my_repr = PyString_FromFormat("%s(%s, variant_level=%ld)",
                                       self->ob_type->tp_name,
@@ -405,8 +409,12 @@ DBusPythonLong_tp_repr(PyObject *self)
 
     if (!parent_repr) return NULL;
     vl_obj = PyObject_GetAttr(self, dbus_py_variant_level_const);
-    if (!vl_obj) return 0;
+    if (!vl_obj) {
+        Py_DECREF(parent_repr);
+        return NULL;
+    }
     variant_level = PyInt_AsLong(vl_obj);
+    Py_DECREF(vl_obj);
     if (variant_level) {
         my_repr = PyString_FromFormat("%s(%s, variant_level=%ld)",
                                       self->ob_type->tp_name,
-- 
1.4.4.4

-------------- next part --------------
>From c04456ff1b24de8695cda14f91e8886ca9c0bf0f Mon Sep 17 00:00:00 2001
From: Simon McVittie <simon.mcvittie at collabora.co.uk>
Date: Mon, 5 Feb 2007 15:18:39 +0000
Subject: [PATCH] Don't leak memory in Struct repr()

---
 _dbus_bindings/containers.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/_dbus_bindings/containers.c b/_dbus_bindings/containers.c
index c9af065..4dc9046 100644
--- a/_dbus_bindings/containers.c
+++ b/_dbus_bindings/containers.c
@@ -505,8 +505,9 @@ static PyObject *
 Struct_tp_repr(PyObject *self)
 {
     PyObject *parent_repr = (PyTuple_Type.tp_repr)((PyObject *)self);
-    PyObject *sig, *sig_repr = NULL;
-    PyObject *vl_obj;
+    PyObject *sig = NULL;
+    PyObject *sig_repr = NULL;
+    PyObject *vl_obj = NULL;
     long variant_level;
     PyObject *my_repr = NULL;
 
@@ -535,7 +536,9 @@ Struct_tp_repr(PyObject *self)
 
 finally:
     Py_XDECREF(parent_repr);
+    Py_XDECREF(sig);
     Py_XDECREF(sig_repr);
+    Py_XDECREF(vl_obj);
     return my_repr;
 }
 
-- 
1.4.4.4

-------------- next part --------------
>From 870227fafd9c976a0354b02aff6052ba24234e91 Mon Sep 17 00:00:00 2001
From: Simon McVittie <simon.mcvittie at collabora.co.uk>
Date: Mon, 5 Feb 2007 16:21:13 +0000
Subject: [PATCH] Close a couple of reference leaks in String (there's another somewhere, but I can't find it)

---
 _dbus_bindings/string.c |   30 ++++++++++++++++++++++--------
 1 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/_dbus_bindings/string.c b/_dbus_bindings/string.c
index d2455cb..096635c 100644
--- a/_dbus_bindings/string.c
+++ b/_dbus_bindings/string.c
@@ -241,20 +241,30 @@ String_tp_new(PyTypeObject *cls, PyObject *args, PyObject *kwargs)
     if (!PyArg_ParseTupleAndKeywords(dbus_py_empty_tuple, kwargs,
                                      "|O!:__new__", argnames,
                                      &PyInt_Type, &variantness)) return NULL;
-    if (!variantness) {
+    if (variantness) {
+        if (PyInt_AS_LONG(variantness) < 0) {
+            PyErr_SetString(PyExc_ValueError,
+                            "variant_level must be non-negative");
+            return NULL;
+        }
+        /* own a reference */
+        Py_INCREF(variantness);
+    }
+    else {
         variantness = PyInt_FromLong(0);
         if (!variantness) return NULL;
     }
-    if (PyInt_AS_LONG(variantness) < 0) {
-        PyErr_SetString(PyExc_ValueError,
-                        "variant_level must be non-negative");
-        return NULL;
-    }
 
     self = (PyUnicode_Type.tp_new)(cls, args, NULL);
     if (self) {
-        PyObject_GenericSetAttr(self, dbus_py_variant_level_const, variantness);
+        if (PyObject_GenericSetAttr(self, dbus_py_variant_level_const,
+                                    variantness) < 0) {
+            Py_DECREF(variantness);
+            Py_DECREF(self);
+            return NULL;
+        }
     }
+    Py_DECREF(variantness);
     return self;
 }
 
@@ -268,8 +278,12 @@ String_tp_repr(PyObject *self)
 
     if (!parent_repr) return NULL;
     vl_obj = PyObject_GetAttr(self, dbus_py_variant_level_const);
-    if (!vl_obj) return NULL;
+    if (!vl_obj) {
+        Py_DECREF(parent_repr);
+        return NULL;
+    }
     variant_level = PyInt_AsLong(vl_obj);
+    Py_DECREF(vl_obj);
     if (variant_level > 0) {
         my_repr = PyString_FromFormat("%s(%s, variant_level=%ld)",
                                       self->ob_type->tp_name,
-- 
1.4.4.4



More information about the dbus mailing list