[3.14] gh-146092: Fix error handling in _BINARY_OP_ADD_FLOAT opcode by vstinner · Pull Request #146119 · python/cpython
If you want to be sure. Write a test where the append fails (you can trigger an artificial failure somehow), and check if there's reference leaks. If there's refleaks, then we should not be setting it to PyStackRef_NULL and main is wrong.
I wrote such test and main code is correct, whereas my 3.14 PR was wrong.
I fixed my 3.14 PR by adding *target_local = PyStackRef_NULL; to the error path.
Here is my patch on top of this PR to test the error path using ./python -m test test_str -m test_leak -R 3:3:
diff --git a/Lib/test/test_str.py b/Lib/test/test_str.py index 2584fbf72d3..9fd24782a5c 100644 --- a/Lib/test/test_str.py +++ b/Lib/test/test_str.py @@ -2784,6 +2784,20 @@ class Bag: o.name2 = 4 self.assertEqual(list(o.__dict__), [name, name2]) + def test_leak(self): + def f(): + left = b"opazkeopakeoakezaijzoe".decode() + right = b"xyz".decode() + try: + left += right + except MemoryError: + left = "MemoryError" + return left + + f() + strings = [f() for _ in range(1024)] + self.assertEqual(strings[-1], "MemoryError") + if __name__ == "__main__": unittest.main() diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 7a33f63a051..18eda799074 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -792,7 +792,13 @@ dummy_func( DEAD(left); PyObject *temp = PyStackRef_AsPyObjectSteal(*target_local); PyObject *right_o = PyStackRef_AsPyObjectSteal(right); - PyUnicode_Append(&temp, right_o); + if (PyUnicode_EqualToUTF8(temp, "opazkeopakeoakezaijzoe") == 0) { + PyUnicode_Append(&temp, right_o); + } + else { + Py_CLEAR(temp); + PyErr_NoMemory(); + } Py_DECREF(right_o); if (temp == NULL) { *target_local = PyStackRef_NULL;