diff --git a/NEWS b/NEWS index 519dc866..c255b8fb 100644 --- a/NEWS +++ b/NEWS @@ -18,6 +18,8 @@ What's new in psycopg 2.4.3 - Fixed interaction between RealDictCursor and named cursors (ticket #67). - Dropped limit on the columns length in COPY operations (ticket #68). + - Fixed reference leak with arguments referenced more than once + in queries (ticket #81). - Fixed typecasting of arrays containing consecutive backslashes. - 'errorcodes' map updated to PostgreSQL 9.1. diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index b06371a3..5c609c80 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -123,23 +123,29 @@ _mogrify(PyObject *var, PyObject *fmt, cursorObject *curs, PyObject **new) for (d = c + 1; *d && *d != ')' && *d != '%'; d++); if (*d == ')') { - key = Text_FromUTF8AndSize(c+1, (Py_ssize_t) (d-c-1)); - value = PyObject_GetItem(var, key); - /* key has refcnt 1, value the original value + 1 */ + if (!(key = Text_FromUTF8AndSize(c+1, (Py_ssize_t)(d-c-1)))) { + Py_XDECREF(n); + return -1; + } /* if value is NULL we did not find the key (or this is not a dictionary): let python raise a KeyError */ - if (value == NULL) { + if (!(value = PyObject_GetItem(var, key))) { Py_DECREF(key); /* destroy key */ Py_XDECREF(n); /* destroy n */ return -1; } + /* key has refcnt 1, value the original value + 1 */ Dprintf("_mogrify: value refcnt: " FORMAT_CODE_PY_SSIZE_T " (+1)", Py_REFCNT(value)); if (n == NULL) { - n = PyDict_New(); + if (!(n = PyDict_New())) { + Py_DECREF(key); + Py_DECREF(value); + return -1; + } } if (0 == PyDict_Contains(n, key)) { @@ -156,24 +162,22 @@ _mogrify(PyObject *var, PyObject *fmt, cursorObject *curs, PyObject **new) } else { t = microprotocol_getquoted(value, curs->conn); - if (t != NULL) { PyDict_SetItem(n, key, t); /* both key and t refcnt +1, key is at 2 now */ } else { /* no adapter found, raise a BIG exception */ - Py_XDECREF(value); + Py_DECREF(key); + Py_DECREF(value); Py_DECREF(n); return -1; } } Py_XDECREF(t); /* t dies here */ - /* after the DECREF value has the original refcnt plus 1 - if it was added to the dictionary directly; good */ - Py_XDECREF(value); } + Py_DECREF(value); Py_DECREF(key); /* key has the original refcnt now */ Dprintf("_mogrify: after value refcnt: " FORMAT_CODE_PY_SSIZE_T, Py_REFCNT(value)); diff --git a/tests/test_cursor.py b/tests/test_cursor.py index be097e5f..aaeab139 100755 --- a/tests/test_cursor.py +++ b/tests/test_cursor.py @@ -97,6 +97,18 @@ class CursorTests(unittest.TestCase): self.assertEqual(b('SELECT 10.3;'), cur.mogrify("SELECT %s;", (Decimal("10.3"),))) + def test_mogrify_leak_on_multiple_reference(self): + # issue #81: reference leak when a parameter value is referenced + # more than once from a dict. + cur = self.conn.cursor() + i = lambda x: x + foo = i('foo') * 10 + import sys + nref1 = sys.getrefcount(foo) + cur.mogrify("select %(foo)s, %(foo)s, %(foo)s", {'foo': foo}) + nref2 = sys.getrefcount(foo) + self.assertEqual(nref1, nref2) + def test_bad_placeholder(self): cur = self.conn.cursor() self.assertRaises(psycopg2.ProgrammingError,