- [bug] Fixed memory leak in C version of

result proxy whereby DBAPIs which don't deliver
pure Python tuples for result rows would
fail to decrement refcounts correctly.
The most prominently affected DBAPI
is pyodbc.  [ticket:2489]

diff --git a/CHANGES b/CHANGES
index f6f1e5f392098b99dcb8d6cb8126f6a6cfae41ee..6cbaf125bf8c4b761971967c75d6f171a529161c 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -24,6 +24,14 @@ CHANGES
     to [ticket:1892] as this was supposed
     to be part of that, this is [ticket:2491].
 
+- engine
+  - [bug] Fixed memory leak in C version of
+    result proxy whereby DBAPIs which don't deliver
+    pure Python tuples for result rows would
+    fail to decrement refcounts correctly.
+    The most prominently affected DBAPI
+    is pyodbc.  [ticket:2489]
+
 - oracle
   - [bug] Added ROWID to oracle.*, [ticket:2483]
 

diff --git a/lib/sqlalchemy/cextension/resultproxy.c b/lib/sqlalchemy/cextension/resultproxy.c
index 3494ccae65668c1dc2ae63cd585a0fbb7ef0297d..518209e54ea9933ed5b0f4540fa73896f75b33da 100644 (file)
--- a/lib/sqlalchemy/cextension/resultproxy.c
+++ b/lib/sqlalchemy/cextension/resultproxy.c
@@ -241,13 +241,14 @@ static PyObject *
 BaseRowProxy_subscript(BaseRowProxy *self, PyObject *key)
 {
     PyObject *processors, *values;
-    PyObject *processor, *value;
+    PyObject *processor, *value, *processed_value;
     PyObject *row, *record, *result, *indexobject;
     PyObject *exc_module, *exception;
     char *cstr_key;
     long index;
     int key_fallback = 0;
-
+    int tuple_check = 0;
+    
     if (PyInt_CheckExact(key)) {
         index = PyInt_AS_LONG(key);
     } else if (PyLong_CheckExact(key)) {
@@ -319,17 +320,28 @@ BaseRowProxy_subscript(BaseRowProxy *self, PyObject *key)
         return NULL;
 
     row = self->row;
-    if (PyTuple_CheckExact(row))
+    if (PyTuple_CheckExact(row)) {
         value = PyTuple_GetItem(row, index);
-    else
+        tuple_check = 1;
+    }
+    else {
         value = PySequence_GetItem(row, index);
+        tuple_check = 0;
+    }
+        
     if (value == NULL)
         return NULL;
 
     if (processor != Py_None) {
-        return PyObject_CallFunctionObjArgs(processor, value, NULL);
+        processed_value = PyObject_CallFunctionObjArgs(processor, value, NULL);
+        if (!tuple_check) {
+            Py_DECREF(value);
+        }
+        return processed_value;
     } else {
-        Py_INCREF(value);
+        if (tuple_check) {
+            Py_INCREF(value);
+        }
         return value;
     }
 }

diff --git a/test/aaa_profiling/test_resultset.py b/test/aaa_profiling/test_resultset.py
index f1358b748070e073ea611a56d3fa1b3ebfa9516b..64fa27de00d2a0b2e09df535d21f31499e12b49f 100644 (file)
--- a/test/aaa_profiling/test_resultset.py
+++ b/test/aaa_profiling/test_resultset.py
@@ -1,5 +1,6 @@
 from sqlalchemy import *
 from test.lib import *
+from test.lib.testing import eq_
 NUM_FIELDS = 10
 NUM_RECORDS = 1000
 
@@ -93,3 +94,60 @@ class ExecutionTest(fixtures.TestBase):
             e.execute("select 1")
         go()
 
+
+class RowProxyTest(fixtures.TestBase):
+    def _rowproxy_fixture(self, keys, processors, row):
+        from sqlalchemy.engine.base import RowProxy
+        class MockMeta(object):
+            def __init__(self):
+                pass
+
+        metadata = MockMeta()
+
+        keymap = {}
+        for index, (keyobjs, processor, values) in \
+            enumerate(zip(keys, processors, row)):
+            for key in keyobjs:
+                keymap[key] = (processor, key, index)
+            keymap[index] = (processor, key, index)
+        return RowProxy(metadata, row, processors, keymap)
+
+    def _test_getitem_value_refcounts(self, seq_factory):
+        import sys
+        col1, col2 = object(), object()
+        def proc1(value):
+            return value
+        value1, value2 = "x", "y"
+        row = self._rowproxy_fixture(
+            [(col1, "a"),(col2, "b")],
+            [proc1, None],
+            seq_factory([value1, value2])
+        )
+
+        v1_refcount = sys.getrefcount(value1)
+        v2_refcount = sys.getrefcount(value2)
+        for i in range(10):
+            row[col1]
+            row["a"]
+            row[col2]
+            row["b"]
+            row[0]
+            row[1]
+            row[0:2]
+        eq_(sys.getrefcount(value1), v1_refcount)
+        eq_(sys.getrefcount(value2), v2_refcount)
+
+    def test_value_refcounts_pure_tuple(self):
+        self._test_getitem_value_refcounts(tuple)
+
+    def test_value_refcounts_custom_seq(self):
+        class CustomSeq(object):
+            def __init__(self, data):
+                self.data = data
+
+            def __getitem__(self, item):
+                return self.data[item]
+
+            def __iter__(self):
+                return iter(self.data)
+        self._test_getitem_value_refcounts(CustomSeq)