Fixed reference leak with arguments referenced more than once in queries

Plus, some more care in objects life cycle, mostly in exceptions handling.

Closes ticket #81.

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.
 

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));
+                if (!(key = Text_FromUTF8AndSize(c+1, (Py_ssize_t)(d-c-1)))) {
-                value = PyObject_GetItem(var, key);
+                    Py_XDECREF(n);
-                /* key has refcnt 1, value the original value + 1 */
+                    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));

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,