From 89d406218fa438a3c330732d971af5519738bc02 Mon Sep 17 00:00:00 2001 From: Mikhail Efimov Date: Fri, 24 Oct 2025 16:30:38 +0300 Subject: [PATCH 1/3] Leak fixed --- Lib/test/test_builtin.py | 17 +++++++++++++++++ Python/bltinmodule.c | 38 +++++++++++++++++++++----------------- 2 files changed, 38 insertions(+), 17 deletions(-) diff --git a/Lib/test/test_builtin.py b/Lib/test/test_builtin.py index 034cd5f7f9e89f..e9cbde4cc26fe5 100644 --- a/Lib/test/test_builtin.py +++ b/Lib/test/test_builtin.py @@ -1457,6 +1457,23 @@ def __next__(self): l8 = self.iter_error(map(pack, Iter(3), "AB", strict=True), ValueError) self.assertEqual(l8, [(2, "A"), (1, "B")]) + # gh-140517: fix refcount leak + def test_map_strict_noleak(self): + t1 = (None, object()) + t2 = (object(), object()) + t3 = (object(),) + + self.assertRaises(ValueError, tuple, + map(pack, t1, 'a', strict=True)) + self.assertRaises(ValueError, tuple, + map(pack, t1, t2, 'a', strict=True)) + self.assertRaises(ValueError, tuple, + map(pack, t1, t2, t3, strict=True)) + self.assertRaises(ValueError, tuple, + map(pack, 'a', t1, strict=True)) + self.assertRaises(ValueError, tuple, + map(pack, 'a', t2, t3, strict=True)) + def test_max(self): self.assertEqual(max('123123'), '3') self.assertEqual(max(1, 2, 3), 3) diff --git a/Python/bltinmodule.c b/Python/bltinmodule.c index 64249177eec5f2..c7f68497fa33e1 100644 --- a/Python/bltinmodule.c +++ b/Python/bltinmodule.c @@ -1515,20 +1515,14 @@ map_next(PyObject *self) } result = _PyObject_VectorcallTstate(tstate, lz->func, stack, nargs, NULL); + goto exit; -exit: - for (i=0; i < nargs; i++) { - Py_DECREF(stack[i]); - } - if (stack != small_stack) { - PyMem_Free(stack); - } - return result; check: if (PyErr_Occurred()) { if (!PyErr_ExceptionMatches(PyExc_StopIteration)) { // next() on argument i raised an exception (not StopIteration) - return NULL; + // result is NULL; + goto exit; } PyErr_Clear(); } @@ -1536,9 +1530,10 @@ map_next(PyObject *self) // ValueError: map() argument 2 is shorter than argument 1 // ValueError: map() argument 3 is shorter than arguments 1-2 const char* plural = i == 1 ? " " : "s 1-"; - return PyErr_Format(PyExc_ValueError, - "map() argument %d is shorter than argument%s%d", - i + 1, plural, i); + result = PyErr_Format(PyExc_ValueError, + "map() argument %d is shorter than argument%s%d", + i + 1, plural, i); + goto exit; } for (i = 1; i < niters; i++) { PyObject *it = PyTuple_GET_ITEM(lz->iters, i); @@ -1546,21 +1541,30 @@ map_next(PyObject *self) if (val) { Py_DECREF(val); const char* plural = i == 1 ? " " : "s 1-"; - return PyErr_Format(PyExc_ValueError, - "map() argument %d is longer than argument%s%d", - i + 1, plural, i); + result = PyErr_Format(PyExc_ValueError, + "map() argument %d is longer than argument%s%d", + i + 1, plural, i); + goto exit; } if (PyErr_Occurred()) { if (!PyErr_ExceptionMatches(PyExc_StopIteration)) { // next() on argument i raised an exception (not StopIteration) - return NULL; + // result is NULL; + goto exit; } PyErr_Clear(); } // Argument i is exhausted. So far so good... } // All arguments are exhausted. Success! - goto exit; +exit: + for (i=0; i < nargs; i++) { + Py_DECREF(stack[i]); + } + if (stack != small_stack) { + PyMem_Free(stack); + } + return result; } static PyObject * From 73481115f3261ad8d22e403d2429932db621dc5a Mon Sep 17 00:00:00 2001 From: Mikhail Efimov Date: Fri, 24 Oct 2025 17:01:46 +0300 Subject: [PATCH 2/3] Review addressed --- Lib/test/test_builtin.py | 32 +++++++++++++++----------------- Python/bltinmodule.c | 6 ++++-- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/Lib/test/test_builtin.py b/Lib/test/test_builtin.py index e9cbde4cc26fe5..eaa8100eb7c2c4 100644 --- a/Lib/test/test_builtin.py +++ b/Lib/test/test_builtin.py @@ -1385,6 +1385,21 @@ def test_map_strict(self): self.assertRaises(ValueError, tuple, map(pack, (1, 2), (1, 2), 'abc', strict=True)) + t1 = (None, object()) + t2 = (object(), object()) + t3 = (object(),) + + self.assertRaises(ValueError, tuple, + map(pack, t1, 'a', strict=True)) + self.assertRaises(ValueError, tuple, + map(pack, t1, t2, 'a', strict=True)) + self.assertRaises(ValueError, tuple, + map(pack, t1, t2, t3, strict=True)) + self.assertRaises(ValueError, tuple, + map(pack, 'a', t1, strict=True)) + self.assertRaises(ValueError, tuple, + map(pack, 'a', t2, t3, strict=True)) + def test_map_strict_iterators(self): x = iter(range(5)) y = [0] @@ -1457,23 +1472,6 @@ def __next__(self): l8 = self.iter_error(map(pack, Iter(3), "AB", strict=True), ValueError) self.assertEqual(l8, [(2, "A"), (1, "B")]) - # gh-140517: fix refcount leak - def test_map_strict_noleak(self): - t1 = (None, object()) - t2 = (object(), object()) - t3 = (object(),) - - self.assertRaises(ValueError, tuple, - map(pack, t1, 'a', strict=True)) - self.assertRaises(ValueError, tuple, - map(pack, t1, t2, 'a', strict=True)) - self.assertRaises(ValueError, tuple, - map(pack, t1, t2, t3, strict=True)) - self.assertRaises(ValueError, tuple, - map(pack, 'a', t1, strict=True)) - self.assertRaises(ValueError, tuple, - map(pack, 'a', t2, t3, strict=True)) - def test_max(self): self.assertEqual(max('123123'), '3') self.assertEqual(max(1, 2, 3), 3) diff --git a/Python/bltinmodule.c b/Python/bltinmodule.c index c7f68497fa33e1..8828e90335a293 100644 --- a/Python/bltinmodule.c +++ b/Python/bltinmodule.c @@ -1521,7 +1521,7 @@ map_next(PyObject *self) if (PyErr_Occurred()) { if (!PyErr_ExceptionMatches(PyExc_StopIteration)) { // next() on argument i raised an exception (not StopIteration) - // result is NULL; + assert(result == NULL); goto exit; } PyErr_Clear(); @@ -1549,7 +1549,7 @@ map_next(PyObject *self) if (PyErr_Occurred()) { if (!PyErr_ExceptionMatches(PyExc_StopIteration)) { // next() on argument i raised an exception (not StopIteration) - // result is NULL; + assert(result == NULL); goto exit; } PyErr_Clear(); @@ -1557,6 +1557,8 @@ map_next(PyObject *self) // Argument i is exhausted. So far so good... } // All arguments are exhausted. Success! + assert(result == NULL); + exit: for (i=0; i < nargs; i++) { Py_DECREF(stack[i]); From 0aadfe15d5f684d3fb989769d8ab29497f738229 Mon Sep 17 00:00:00 2001 From: Mikhail Efimov Date: Fri, 24 Oct 2025 17:21:17 +0300 Subject: [PATCH 3/3] Review addressed 2 --- Lib/test/test_builtin.py | 1 + Python/bltinmodule.c | 30 +++++++++++++++--------------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/Lib/test/test_builtin.py b/Lib/test/test_builtin.py index eaa8100eb7c2c4..fe3e391a7f5ba1 100644 --- a/Lib/test/test_builtin.py +++ b/Lib/test/test_builtin.py @@ -1385,6 +1385,7 @@ def test_map_strict(self): self.assertRaises(ValueError, tuple, map(pack, (1, 2), (1, 2), 'abc', strict=True)) + # gh-140517: Testing refleaks with mortal objects. t1 = (None, object()) t2 = (object(), object()) t3 = (object(),) diff --git a/Python/bltinmodule.c b/Python/bltinmodule.c index 8828e90335a293..f6fadd936bb8ff 100644 --- a/Python/bltinmodule.c +++ b/Python/bltinmodule.c @@ -1501,14 +1501,14 @@ map_next(PyObject *self) } Py_ssize_t nargs = 0; - for (i=0; i < niters; i++) { + for (i = 0; i < niters; i++) { PyObject *it = PyTuple_GET_ITEM(lz->iters, i); PyObject *val = Py_TYPE(it)->tp_iternext(it); if (val == NULL) { if (lz->strict) { goto check; } - goto exit; + goto exit_no_result; } stack[i] = val; nargs++; @@ -1521,8 +1521,7 @@ map_next(PyObject *self) if (PyErr_Occurred()) { if (!PyErr_ExceptionMatches(PyExc_StopIteration)) { // next() on argument i raised an exception (not StopIteration) - assert(result == NULL); - goto exit; + goto exit_no_result; } PyErr_Clear(); } @@ -1530,10 +1529,10 @@ map_next(PyObject *self) // ValueError: map() argument 2 is shorter than argument 1 // ValueError: map() argument 3 is shorter than arguments 1-2 const char* plural = i == 1 ? " " : "s 1-"; - result = PyErr_Format(PyExc_ValueError, - "map() argument %d is shorter than argument%s%d", - i + 1, plural, i); - goto exit; + PyErr_Format(PyExc_ValueError, + "map() argument %d is shorter than argument%s%d", + i + 1, plural, i); + goto exit_no_result; } for (i = 1; i < niters; i++) { PyObject *it = PyTuple_GET_ITEM(lz->iters, i); @@ -1541,26 +1540,27 @@ map_next(PyObject *self) if (val) { Py_DECREF(val); const char* plural = i == 1 ? " " : "s 1-"; - result = PyErr_Format(PyExc_ValueError, - "map() argument %d is longer than argument%s%d", - i + 1, plural, i); - goto exit; + PyErr_Format(PyExc_ValueError, + "map() argument %d is longer than argument%s%d", + i + 1, plural, i); + goto exit_no_result; } if (PyErr_Occurred()) { if (!PyErr_ExceptionMatches(PyExc_StopIteration)) { // next() on argument i raised an exception (not StopIteration) - assert(result == NULL); - goto exit; + goto exit_no_result; } PyErr_Clear(); } // Argument i is exhausted. So far so good... } // All arguments are exhausted. Success! + +exit_no_result: assert(result == NULL); exit: - for (i=0; i < nargs; i++) { + for (i = 0; i < nargs; i++) { Py_DECREF(stack[i]); } if (stack != small_stack) {