From f59e619ab100270896b526d41e9c0faf04285d3f Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Fri, 28 Feb 2025 11:08:20 -0500 Subject: [PATCH 1/7] gh-130382: add missing _PyReftracerTrack to ceval DECREF --- Python/ceval.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Python/ceval.c b/Python/ceval.c index 0a3b30513733bd..7f1ee61251ebc4 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -80,6 +80,7 @@ } \ _Py_DECREF_STAT_INC(); \ if (--op->ob_refcnt == 0) { \ + _PyReftracerTrack(op, PyRefTracer_DESTROY); \ destructor dealloc = Py_TYPE(op)->tp_dealloc; \ (*dealloc)(op); \ } \ From 1fd3e46da6420706ada163062fce841707cb2d8a Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Fri, 28 Feb 2025 16:13:03 +0000 Subject: [PATCH 2/7] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2025-02-28-16-13-02.gh-issue-130382.66VTmy.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2025-02-28-16-13-02.gh-issue-130382.66VTmy.rst diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-02-28-16-13-02.gh-issue-130382.66VTmy.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-02-28-16-13-02.gh-issue-130382.66VTmy.rst new file mode 100644 index 00000000000000..682f6dcfa50310 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-02-28-16-13-02.gh-issue-130382.66VTmy.rst @@ -0,0 +1 @@ +Fix ``PyRefTracer_DESTROY`` not being sent from :file:`Python/cevalc` ``Py_DECREF()``. From 741566867dc86285870b75f2f4a56c0f97675b5b Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Fri, 28 Feb 2025 11:25:30 -0500 Subject: [PATCH 3/7] fix news --- .../2025-02-28-16-13-02.gh-issue-130382.66VTmy.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-02-28-16-13-02.gh-issue-130382.66VTmy.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-02-28-16-13-02.gh-issue-130382.66VTmy.rst index 682f6dcfa50310..8b775c8fe1cf40 100644 --- a/Misc/NEWS.d/next/Core_and_Builtins/2025-02-28-16-13-02.gh-issue-130382.66VTmy.rst +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-02-28-16-13-02.gh-issue-130382.66VTmy.rst @@ -1 +1 @@ -Fix ``PyRefTracer_DESTROY`` not being sent from :file:`Python/cevalc` ``Py_DECREF()``. +Fix ``PyRefTracer_DESTROY`` not being sent from :file:`Python/ceval.c` ``Py_DECREF()``. From 9880687438d490aba551c56141a3bcdae165dc8b Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Tue, 4 Mar 2025 09:06:36 -0500 Subject: [PATCH 4/7] add test using _testcapimodule --- Lib/test/test_capi/test_misc.py | 21 +++++++++++++++++ Modules/_testcapimodule.c | 42 +++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py index 6c4cf5bd6598bc..b2e77454491590 100644 --- a/Lib/test/test_capi/test_misc.py +++ b/Lib/test/test_capi/test_misc.py @@ -2884,5 +2884,26 @@ def test_pack_version_ctypes(self): self.assertEqual(result, expected) +class TestCEval(unittest.TestCase): + def test_ceval_decref(self): + def f(): + l = [] + del l + + def g(): + l = [], [] + del l + + _testcapi.toggle_reftrace_counter() + f() + refs = _testcapi.toggle_reftrace_counter() + self.assertEqual(refs, (1, 1)) + + _testcapi.toggle_reftrace_counter() + g() + refs = _testcapi.toggle_reftrace_counter() + self.assertEqual(refs, (3, 3)) + + if __name__ == "__main__": unittest.main() diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 9260c7659440ea..2f7a0554e06e79 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -2514,6 +2514,47 @@ code_offset_to_line(PyObject* self, PyObject* const* args, Py_ssize_t nargsf) return PyLong_FromInt32(PyCode_Addr2Line(code, offset)); } +typedef struct { + Py_ssize_t create_count; + Py_ssize_t destroy_count; + void *prev_data; + int (*prev_tracer)(PyObject *, PyRefTracerEvent, void *); +} reftrace_counter_data; + +static int _reftrace_counter(PyObject *obj, PyRefTracerEvent event, void *counter_data) +{ + if (event == PyRefTracer_CREATE) { + ((reftrace_counter_data *)counter_data)->create_count++; + } else { + ((reftrace_counter_data *)counter_data)->destroy_count++; + } + return 0; +} + +// A very simple reftrace counter for very simple tests +static PyObject * +toggle_reftrace_counter(PyObject *ob, PyObject *Py_UNUSED(args)) +{ + static reftrace_counter_data counter_data = {0, 0, NULL, (void *)toggle_reftrace_counter}; // sentinel + + if (counter_data.prev_tracer == (void *)toggle_reftrace_counter) { + // toggle counter on + counter_data.create_count = counter_data.destroy_count = 0; + counter_data.prev_tracer = PyRefTracer_GetTracer(&counter_data.prev_data); + + PyRefTracer_SetTracer(_reftrace_counter, &counter_data); + + Py_RETURN_NONE; + } + + // toggle counter off + PyRefTracer_SetTracer(counter_data.prev_tracer, counter_data.prev_data); + + counter_data.prev_tracer = (void *)toggle_reftrace_counter; + + return Py_BuildValue("(nn)", counter_data.create_count, counter_data.destroy_count); +} + static PyMethodDef TestMethods[] = { {"set_errno", set_errno, METH_VARARGS}, @@ -2608,6 +2649,7 @@ static PyMethodDef TestMethods[] = { {"finalize_thread_hang", finalize_thread_hang, METH_O, NULL}, {"test_atexit", test_atexit, METH_NOARGS}, {"code_offset_to_line", _PyCFunction_CAST(code_offset_to_line), METH_FASTCALL}, + {"toggle_reftrace_counter", toggle_reftrace_counter, METH_NOARGS}, {NULL, NULL} /* sentinel */ }; From d44a362664b49cbd55605b48f720c27bc5c914be Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Tue, 4 Mar 2025 09:19:35 -0500 Subject: [PATCH 5/7] fix format --- Modules/_testcapimodule.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 2f7a0554e06e79..72bcc2797a4918 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -2521,7 +2521,8 @@ typedef struct { int (*prev_tracer)(PyObject *, PyRefTracerEvent, void *); } reftrace_counter_data; -static int _reftrace_counter(PyObject *obj, PyRefTracerEvent event, void *counter_data) +static int +_reftrace_counter(PyObject *obj, PyRefTracerEvent event, void *counter_data) { if (event == PyRefTracer_CREATE) { ((reftrace_counter_data *)counter_data)->create_count++; From 78cac866916303044b77c377f4e76dddcfb18369 Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Tue, 4 Mar 2025 12:11:52 -0500 Subject: [PATCH 6/7] this test is starting to get silly --- Lib/test/test_capi/test_misc.py | 6 ++- Modules/_testcapimodule.c | 84 ++++++++++++++++++++++++++++----- 2 files changed, 76 insertions(+), 14 deletions(-) diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py index b2e77454491590..6dcdf3c5558532 100644 --- a/Lib/test/test_capi/test_misc.py +++ b/Lib/test/test_capi/test_misc.py @@ -2897,12 +2897,14 @@ def g(): _testcapi.toggle_reftrace_counter() f() refs = _testcapi.toggle_reftrace_counter() - self.assertEqual(refs, (1, 1)) + # sometimes we get a stray DECREF from somewhere else (other thread?) + # doesn't happen outside of test + self.assertIn(refs, ((1, 1), (1, 2))) _testcapi.toggle_reftrace_counter() g() refs = _testcapi.toggle_reftrace_counter() - self.assertEqual(refs, (3, 3)) + self.assertIn(refs, ((3, 3), (3, 4))) if __name__ == "__main__": diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 72bcc2797a4918..5a0796f4ebdb69 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -2514,6 +2514,7 @@ code_offset_to_line(PyObject* self, PyObject* const* args, Py_ssize_t nargsf) return PyLong_FromInt32(PyCode_Addr2Line(code, offset)); } + typedef struct { Py_ssize_t create_count; Py_ssize_t destroy_count; @@ -2526,34 +2527,93 @@ _reftrace_counter(PyObject *obj, PyRefTracerEvent event, void *counter_data) { if (event == PyRefTracer_CREATE) { ((reftrace_counter_data *)counter_data)->create_count++; - } else { + } + else { // PyRefTracer_DESTROY ((reftrace_counter_data *)counter_data)->destroy_count++; } return 0; } -// A very simple reftrace counter for very simple tests +void +reftrace_counter_capsule_destructor(PyObject *capsule) +{ + reftrace_counter_data *counter_data = (reftrace_counter_data *)PyCapsule_GetPointer(capsule, "counter_data"); + + if (counter_data) { + if (counter_data->prev_tracer != (void *)_reftrace_counter) { + // juuust in case, to not segfault + PyRefTracer_SetTracer(counter_data->prev_tracer, counter_data->prev_data); + } + + PyMem_Free(counter_data); + } +} + +// A simple reftrace counter for very simple tests static PyObject * -toggle_reftrace_counter(PyObject *ob, PyObject *Py_UNUSED(args)) +toggle_reftrace_counter(PyObject *ob, PyObject *args) { - static reftrace_counter_data counter_data = {0, 0, NULL, (void *)toggle_reftrace_counter}; // sentinel + reftrace_counter_data *counter_data; + PyObject *capsule; + + if (PyObject_GetOptionalAttrString(ob, "_reftrace_counter_data", &capsule)) { + if (!PyCapsule_IsValid(capsule, "counter_data")) { + PyErr_SetString(PyExc_ValueError, "something went wrong, _reftrace_counter_data is invalid"); + return NULL; + } + + counter_data = PyCapsule_GetPointer(capsule, "counter_data"); + + if (counter_data == NULL) { + PyErr_SetString(PyExc_ValueError, "something went wrong, _reftrace_counter_data is invalid"); + return NULL; + } + } + else { + counter_data = (reftrace_counter_data *)PyMem_Malloc(sizeof(reftrace_counter_data)); - if (counter_data.prev_tracer == (void *)toggle_reftrace_counter) { + if (counter_data == NULL) { + PyErr_NoMemory(); + return NULL; + } + + capsule = PyCapsule_New(counter_data, "counter_data", reftrace_counter_capsule_destructor); + + if (capsule == NULL) { + PyMem_Free(counter_data); + return NULL; + } + + counter_data->prev_tracer = (void *)_reftrace_counter; // sentinel + + if (PyModule_Add(ob, "_reftrace_counter_data", capsule)) { + return NULL; + } + + Py_INCREF(capsule); + } + + if (counter_data->prev_tracer == (void *)_reftrace_counter) { // toggle counter on - counter_data.create_count = counter_data.destroy_count = 0; - counter_data.prev_tracer = PyRefTracer_GetTracer(&counter_data.prev_data); + counter_data->create_count = counter_data->destroy_count = 0; + counter_data->prev_tracer = PyRefTracer_GetTracer(&counter_data->prev_data); - PyRefTracer_SetTracer(_reftrace_counter, &counter_data); + PyRefTracer_SetTracer(_reftrace_counter, counter_data); + Py_DECREF(capsule); Py_RETURN_NONE; } // toggle counter off - PyRefTracer_SetTracer(counter_data.prev_tracer, counter_data.prev_data); + PyRefTracer_SetTracer(counter_data->prev_tracer, counter_data->prev_data); + + Py_ssize_t create_count = counter_data->create_count - 1; // because of PyObject_GetOptionalAttrString + Py_ssize_t destroy_count = counter_data->destroy_count; + counter_data->prev_tracer = (void *)_reftrace_counter; - counter_data.prev_tracer = (void *)toggle_reftrace_counter; + Py_DECREF(capsule); - return Py_BuildValue("(nn)", counter_data.create_count, counter_data.destroy_count); + return Py_BuildValue("(nn)", create_count, destroy_count); } @@ -2650,7 +2710,7 @@ static PyMethodDef TestMethods[] = { {"finalize_thread_hang", finalize_thread_hang, METH_O, NULL}, {"test_atexit", test_atexit, METH_NOARGS}, {"code_offset_to_line", _PyCFunction_CAST(code_offset_to_line), METH_FASTCALL}, - {"toggle_reftrace_counter", toggle_reftrace_counter, METH_NOARGS}, + {"toggle_reftrace_counter", toggle_reftrace_counter, METH_VARARGS}, {NULL, NULL} /* sentinel */ }; From e0827a56d534e4d1d85444ec891672ee8d0ea684 Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Wed, 5 Mar 2025 06:57:13 -0500 Subject: [PATCH 7/7] much better --- Lib/test/test_capi/test_misc.py | 32 +++++------ Modules/_testcapimodule.c | 97 ++++----------------------------- 2 files changed, 23 insertions(+), 106 deletions(-) diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py index 6dcdf3c5558532..2e1e8e77d620d1 100644 --- a/Lib/test/test_capi/test_misc.py +++ b/Lib/test/test_capi/test_misc.py @@ -2886,25 +2886,19 @@ def test_pack_version_ctypes(self): class TestCEval(unittest.TestCase): def test_ceval_decref(self): - def f(): - l = [] - del l - - def g(): - l = [], [] - del l - - _testcapi.toggle_reftrace_counter() - f() - refs = _testcapi.toggle_reftrace_counter() - # sometimes we get a stray DECREF from somewhere else (other thread?) - # doesn't happen outside of test - self.assertIn(refs, ((1, 1), (1, 2))) - - _testcapi.toggle_reftrace_counter() - g() - refs = _testcapi.toggle_reftrace_counter() - self.assertIn(refs, ((3, 3), (3, 4))) + code = textwrap.dedent(""" + import _testcapi + _testcapi.toggle_reftrace_printer(True) + l1 = [] + l2 = [] + del l1 + del l2 + _testcapi.toggle_reftrace_printer(False) + """) + _, out, _ = assert_python_ok("-c", code) + lines = out.decode("utf-8").splitlines() + self.assertEqual(lines.count("CREATE list"), 2) + self.assertEqual(lines.count("DESTROY list"), 2) if __name__ == "__main__": diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 5a0796f4ebdb69..409dd02e1435b5 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -2515,108 +2515,31 @@ code_offset_to_line(PyObject* self, PyObject* const* args, Py_ssize_t nargsf) } -typedef struct { - Py_ssize_t create_count; - Py_ssize_t destroy_count; - void *prev_data; - int (*prev_tracer)(PyObject *, PyRefTracerEvent, void *); -} reftrace_counter_data; - static int -_reftrace_counter(PyObject *obj, PyRefTracerEvent event, void *counter_data) +_reftrace_printer(PyObject *obj, PyRefTracerEvent event, void *counter_data) { if (event == PyRefTracer_CREATE) { - ((reftrace_counter_data *)counter_data)->create_count++; + printf("CREATE %s\n", Py_TYPE(obj)->tp_name); } else { // PyRefTracer_DESTROY - ((reftrace_counter_data *)counter_data)->destroy_count++; + printf("DESTROY %s\n", Py_TYPE(obj)->tp_name); } return 0; } -void -reftrace_counter_capsule_destructor(PyObject *capsule) -{ - reftrace_counter_data *counter_data = (reftrace_counter_data *)PyCapsule_GetPointer(capsule, "counter_data"); - - if (counter_data) { - if (counter_data->prev_tracer != (void *)_reftrace_counter) { - // juuust in case, to not segfault - PyRefTracer_SetTracer(counter_data->prev_tracer, counter_data->prev_data); - } - - PyMem_Free(counter_data); - } -} - -// A simple reftrace counter for very simple tests +// A simple reftrace printer for very simple tests static PyObject * -toggle_reftrace_counter(PyObject *ob, PyObject *args) +toggle_reftrace_printer(PyObject *ob, PyObject *arg) { - reftrace_counter_data *counter_data; - PyObject *capsule; - - if (PyObject_GetOptionalAttrString(ob, "_reftrace_counter_data", &capsule)) { - if (!PyCapsule_IsValid(capsule, "counter_data")) { - PyErr_SetString(PyExc_ValueError, "something went wrong, _reftrace_counter_data is invalid"); - return NULL; - } - - counter_data = PyCapsule_GetPointer(capsule, "counter_data"); - - if (counter_data == NULL) { - PyErr_SetString(PyExc_ValueError, "something went wrong, _reftrace_counter_data is invalid"); - return NULL; - } + if (arg == Py_True) { + PyRefTracer_SetTracer(_reftrace_printer, NULL); } else { - counter_data = (reftrace_counter_data *)PyMem_Malloc(sizeof(reftrace_counter_data)); - - if (counter_data == NULL) { - PyErr_NoMemory(); - return NULL; - } - - capsule = PyCapsule_New(counter_data, "counter_data", reftrace_counter_capsule_destructor); - - if (capsule == NULL) { - PyMem_Free(counter_data); - return NULL; - } - - counter_data->prev_tracer = (void *)_reftrace_counter; // sentinel - - if (PyModule_Add(ob, "_reftrace_counter_data", capsule)) { - return NULL; - } - - Py_INCREF(capsule); - } - - if (counter_data->prev_tracer == (void *)_reftrace_counter) { - // toggle counter on - counter_data->create_count = counter_data->destroy_count = 0; - counter_data->prev_tracer = PyRefTracer_GetTracer(&counter_data->prev_data); - - PyRefTracer_SetTracer(_reftrace_counter, counter_data); - - Py_DECREF(capsule); - Py_RETURN_NONE; + PyRefTracer_SetTracer(NULL, NULL); } - - // toggle counter off - PyRefTracer_SetTracer(counter_data->prev_tracer, counter_data->prev_data); - - Py_ssize_t create_count = counter_data->create_count - 1; // because of PyObject_GetOptionalAttrString - Py_ssize_t destroy_count = counter_data->destroy_count; - counter_data->prev_tracer = (void *)_reftrace_counter; - - Py_DECREF(capsule); - - return Py_BuildValue("(nn)", create_count, destroy_count); + Py_RETURN_NONE; } - static PyMethodDef TestMethods[] = { {"set_errno", set_errno, METH_VARARGS}, {"test_config", test_config, METH_NOARGS}, @@ -2710,7 +2633,7 @@ static PyMethodDef TestMethods[] = { {"finalize_thread_hang", finalize_thread_hang, METH_O, NULL}, {"test_atexit", test_atexit, METH_NOARGS}, {"code_offset_to_line", _PyCFunction_CAST(code_offset_to_line), METH_FASTCALL}, - {"toggle_reftrace_counter", toggle_reftrace_counter, METH_VARARGS}, + {"toggle_reftrace_printer", toggle_reftrace_printer, METH_O}, {NULL, NULL} /* sentinel */ };