From cd676cd8dfbddf1440849f96fa0cdabd4fd72497 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Fri, 1 Jun 2012 16:31:00 -0400 Subject: [PATCH] - [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] --- CHANGES | 8 ++++ lib/sqlalchemy/cextension/resultproxy.c | 24 +++++++--- test/aaa_profiling/test_resultset.py | 58 +++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 6 deletions(-) diff --git a/CHANGES b/CHANGES index f6f1e5f392..6cbaf125bf 100644 --- 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 3494ccae65..518209e54e 100644 --- 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 f1358b7480..64fa27de00 100644 --- 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) -- 2.47.2