From 2b97e61a7b298e4df444acb2725ab76ebb16c02b Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sun, 15 Sep 2019 10:19:20 +0300 Subject: [PATCH 1/2] bpo-38175: Fix a memory leak in comparison of sqlite3.Row objects. --- Lib/sqlite3/test/factory.py | 28 ++++++++++++++----- .../2019-09-15-10-30-33.bpo-38175.61XlUv.rst | 1 + Modules/_sqlite/row.c | 15 ++++++---- 3 files changed, 31 insertions(+), 13 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2019-09-15-10-30-33.bpo-38175.61XlUv.rst diff --git a/Lib/sqlite3/test/factory.py b/Lib/sqlite3/test/factory.py index ced8445536e3c69..f103211ed9f1958 100644 --- a/Lib/sqlite3/test/factory.py +++ b/Lib/sqlite3/test/factory.py @@ -169,19 +169,33 @@ def CheckSqliteRowHashCmp(self): row_1 = self.con.execute("select 1 as a, 2 as b").fetchone() row_2 = self.con.execute("select 1 as a, 2 as b").fetchone() row_3 = self.con.execute("select 1 as a, 3 as b").fetchone() + row_4 = self.con.execute("select 1 as b, 2 as a").fetchone() + row_5 = self.con.execute("select 2 as b, 1 as a").fetchone() - self.assertEqual(row_1, row_1) - self.assertEqual(row_1, row_2) - self.assertTrue(row_2 != row_3) + self.assertTrue(row_1 == row_1) + self.assertTrue(row_1 == row_2) + self.assertFalse(row_1 == row_3) + self.assertFalse(row_1 == row_4) + self.assertFalse(row_1 == row_5) + self.assertFalse(row_1 == object()) self.assertFalse(row_1 != row_1) self.assertFalse(row_1 != row_2) - self.assertFalse(row_2 == row_3) + self.assertTrue(row_1 != row_3) + self.assertTrue(row_1 != row_4) + self.assertTrue(row_1 != row_5) + self.assertTrue(row_1 != object()) + + with self.assertRaises(TypeError): + row_1 > row_2 + with self.assertRaises(TypeError): + row_1 < row_2 + with self.assertRaises(TypeError): + row_1 >= row_2 + with self.assertRaises(TypeError): + row_1 <= row_2 - self.assertEqual(row_1, row_2) self.assertEqual(hash(row_1), hash(row_2)) - self.assertNotEqual(row_1, row_3) - self.assertNotEqual(hash(row_1), hash(row_3)) def CheckSqliteRowAsSequence(self): """ Checks if the row object can act like a sequence """ diff --git a/Misc/NEWS.d/next/Library/2019-09-15-10-30-33.bpo-38175.61XlUv.rst b/Misc/NEWS.d/next/Library/2019-09-15-10-30-33.bpo-38175.61XlUv.rst new file mode 100644 index 000000000000000..120768f1012f72d --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-09-15-10-30-33.bpo-38175.61XlUv.rst @@ -0,0 +1 @@ +Fix da memory leak in comparison of :class:`sqlite3.Row` objects. diff --git a/Modules/_sqlite/row.c b/Modules/_sqlite/row.c index 5c2f40082402e30..844e951ec05a6da 100644 --- a/Modules/_sqlite/row.c +++ b/Modules/_sqlite/row.c @@ -187,19 +187,22 @@ static Py_hash_t pysqlite_row_hash(pysqlite_Row *self) return PyObject_Hash(self->description) ^ PyObject_Hash(self->data); } -static PyObject* pysqlite_row_richcompare(pysqlite_Row *self, PyObject *_other, int opid) +static PyObject * +pysqlite_row_richcompare(pysqlite_Row *self, PyObject *_other, int opid) { if (opid != Py_EQ && opid != Py_NE) Py_RETURN_NOTIMPLEMENTED; - if (PyType_IsSubtype(Py_TYPE(_other), &pysqlite_RowType)) { + if (PyObject_TypeCheck(_other, &pysqlite_RowType)) { pysqlite_Row *other = (pysqlite_Row *)_other; - PyObject *res = PyObject_RichCompare(self->description, other->description, opid); - if ((opid == Py_EQ && res == Py_True) - || (opid == Py_NE && res == Py_False)) { - Py_DECREF(res); + int eq = PyObject_RichCompareBool(self->description, other->description, Py_EQ); + if (eq < 0) { + return NULL; + } + if (eq) { return PyObject_RichCompare(self->data, other->data, opid); } + return PyBool_FromLong(opid != Py_EQ); } Py_RETURN_NOTIMPLEMENTED; } From 92aad907c19f8f2775182d07c6bf62f7d5b6b889 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Mon, 16 Sep 2019 19:36:05 +0300 Subject: [PATCH 2/2] Address review comments. --- .../next/Library/2019-09-15-10-30-33.bpo-38175.61XlUv.rst | 2 +- Modules/_sqlite/row.c | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2019-09-15-10-30-33.bpo-38175.61XlUv.rst b/Misc/NEWS.d/next/Library/2019-09-15-10-30-33.bpo-38175.61XlUv.rst index 120768f1012f72d..6d9f280bbd050a3 100644 --- a/Misc/NEWS.d/next/Library/2019-09-15-10-30-33.bpo-38175.61XlUv.rst +++ b/Misc/NEWS.d/next/Library/2019-09-15-10-30-33.bpo-38175.61XlUv.rst @@ -1 +1 @@ -Fix da memory leak in comparison of :class:`sqlite3.Row` objects. +Fix a memory leak in comparison of :class:`sqlite3.Row` objects. diff --git a/Modules/_sqlite/row.c b/Modules/_sqlite/row.c index 844e951ec05a6da..758518a8ff80be4 100644 --- a/Modules/_sqlite/row.c +++ b/Modules/_sqlite/row.c @@ -187,8 +187,7 @@ static Py_hash_t pysqlite_row_hash(pysqlite_Row *self) return PyObject_Hash(self->description) ^ PyObject_Hash(self->data); } -static PyObject * -pysqlite_row_richcompare(pysqlite_Row *self, PyObject *_other, int opid) +static PyObject* pysqlite_row_richcompare(pysqlite_Row *self, PyObject *_other, int opid) { if (opid != Py_EQ && opid != Py_NE) Py_RETURN_NOTIMPLEMENTED;