From aa88a611610e5f42197409e12a49411c62d6dc62 Mon Sep 17 00:00:00 2001 From: Yukihiro Nakadaira Date: Sun, 11 Dec 2022 14:34:21 +0900 Subject: [PATCH 1/6] gh-99952: [ctypes] fix refcount issues in from_param() result. --- Lib/test/test_ctypes/test_parameters.py | 53 +++++++++++++++++++++++++ Modules/_ctypes/_ctypes.c | 3 ++ 2 files changed, 56 insertions(+) diff --git a/Lib/test/test_ctypes/test_parameters.py b/Lib/test/test_ctypes/test_parameters.py index 84839d9c6a96d9..e598d329b5f0ec 100644 --- a/Lib/test/test_ctypes/test_parameters.py +++ b/Lib/test/test_ctypes/test_parameters.py @@ -1,3 +1,4 @@ +import sys import unittest from test.test_ctypes import need_symbol import test.support @@ -243,6 +244,58 @@ def test_parameter_repr(self): self.assertRegex(repr(c_wchar_p.from_param('hihi')), r"^$") self.assertRegex(repr(c_void_p.from_param(0x12)), r"^$") + @test.support.cpython_only + @unittest.skipUnless(sys.platform == "win32", 'Windows-specific test') + def test_from_param_result_refcount(self): + # Issue #99952 + import sys + import _ctypes_test + from ctypes import PyDLL, c_int, c_void_p, py_object, Structure + + class X(Structure): + _fields_ = [("a", c_void_p)] + + def __del__(self): + trace.append(4) + + @classmethod + def from_param(cls, value): + trace.append(2) + return cls() + + PyList_Append = PyDLL("python", handle=sys.dllhandle).PyList_Append + PyList_Append.restype = c_int + PyList_Append.argtypes = [py_object, py_object, X] + + trace = [] + trace.append(1) + PyList_Append(trace, 3, "dummy") + trace.append(5) + + self.assertEqual(trace, [1, 2, 3, 4, 5]) + + class Y(Structure): + _fields_ = [("a", c_void_p), ("b", c_void_p)] + + def __del__(self): + trace.append(4) + + @classmethod + def from_param(cls, value): + trace.append(2) + return cls() + + PyList_Append = PyDLL("python", handle=sys.dllhandle).PyList_Append + PyList_Append.restype = c_int + PyList_Append.argtypes = [py_object, py_object, Y] + + trace = [] + trace.append(1) + PyList_Append(trace, 3, "dummy") + trace.append(5) + + self.assertEqual(trace, [1, 2, 3, 4, 5]) + ################################################################ if __name__ == '__main__': diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index b9092d3981f364..55bcf044e86928 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -412,6 +412,7 @@ _ctypes_alloc_format_string_with_shape(int ndim, const Py_ssize_t *shape, typedef struct { PyObject_HEAD void *ptr; + PyObject *keep; } StructParamObject; @@ -419,6 +420,7 @@ static void StructParam_dealloc(PyObject *myself) { StructParamObject *self = (StructParamObject *)myself; + Py_XDECREF(self->keep); PyMem_Free(self->ptr); Py_TYPE(self)->tp_free(myself); } @@ -466,6 +468,7 @@ StructUnionType_paramfunc(CDataObject *self) StructParamObject *struct_param = (StructParamObject *)obj; struct_param->ptr = ptr; + struct_param->keep = Py_NewRef(self); } else { ptr = self->b_ptr; obj = Py_NewRef(self); From 6084ba570849eebd9c0a7cd28690daa1cdd3d22b Mon Sep 17 00:00:00 2001 From: Yukihiro Nakadaira Date: Sun, 11 Dec 2022 14:42:05 +0900 Subject: [PATCH 2/6] add blurb --- .../next/Library/2022-12-11-14-38-59.gh-issue-99952.IYGLzr.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2022-12-11-14-38-59.gh-issue-99952.IYGLzr.rst diff --git a/Misc/NEWS.d/next/Library/2022-12-11-14-38-59.gh-issue-99952.IYGLzr.rst b/Misc/NEWS.d/next/Library/2022-12-11-14-38-59.gh-issue-99952.IYGLzr.rst new file mode 100644 index 00000000000000..b05be0c8f02799 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2022-12-11-14-38-59.gh-issue-99952.IYGLzr.rst @@ -0,0 +1 @@ +[ctypes] fix refcount issues in from_param() result. From 2fb99934188f9018014729f7b290fa2e5df3e59a Mon Sep 17 00:00:00 2001 From: Yukihiro Nakadaira Date: Sun, 11 Dec 2022 16:26:59 +0900 Subject: [PATCH 3/6] fix test --- Lib/test/test_ctypes/test_parameters.py | 5 ++--- Modules/_ctypes/_ctypes_test.c | 6 ++++++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_ctypes/test_parameters.py b/Lib/test/test_ctypes/test_parameters.py index e598d329b5f0ec..575700bb6d3b0f 100644 --- a/Lib/test/test_ctypes/test_parameters.py +++ b/Lib/test/test_ctypes/test_parameters.py @@ -245,7 +245,6 @@ def test_parameter_repr(self): self.assertRegex(repr(c_void_p.from_param(0x12)), r"^$") @test.support.cpython_only - @unittest.skipUnless(sys.platform == "win32", 'Windows-specific test') def test_from_param_result_refcount(self): # Issue #99952 import sys @@ -263,7 +262,7 @@ def from_param(cls, value): trace.append(2) return cls() - PyList_Append = PyDLL("python", handle=sys.dllhandle).PyList_Append + PyList_Append = PyDLL(_ctypes_test.__file__)._testfunc_pylist_append PyList_Append.restype = c_int PyList_Append.argtypes = [py_object, py_object, X] @@ -285,7 +284,7 @@ def from_param(cls, value): trace.append(2) return cls() - PyList_Append = PyDLL("python", handle=sys.dllhandle).PyList_Append + PyList_Append = PyDLL(_ctypes_test.__file__)._testfunc_pylist_append PyList_Append.restype = c_int PyList_Append.argtypes = [py_object, py_object, Y] diff --git a/Modules/_ctypes/_ctypes_test.c b/Modules/_ctypes/_ctypes_test.c index e1f91b476a49fd..a8811d03cc91a2 100644 --- a/Modules/_ctypes/_ctypes_test.c +++ b/Modules/_ctypes/_ctypes_test.c @@ -1047,6 +1047,12 @@ EXPORT(long) _test_i38748_runCallback(_test_i38748_funcType callback, int a, int #endif +EXPORT(int) +_testfunc_pylist_append(PyObject *list, PyObject *item) +{ + return PyList_Append(list, item); +} + static struct PyModuleDef_Slot _ctypes_test_slots[] = { {0, NULL} }; From ef884f1781dc1acb4e7cea4ee3f47ed31b4ca091 Mon Sep 17 00:00:00 2001 From: Yukihiro Nakadaira Date: Sun, 11 Dec 2022 16:30:25 +0900 Subject: [PATCH 4/6] fix test --- Lib/test/test_ctypes/test_parameters.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/Lib/test/test_ctypes/test_parameters.py b/Lib/test/test_ctypes/test_parameters.py index 575700bb6d3b0f..eaef3e76acea9a 100644 --- a/Lib/test/test_ctypes/test_parameters.py +++ b/Lib/test/test_ctypes/test_parameters.py @@ -1,4 +1,3 @@ -import sys import unittest from test.test_ctypes import need_symbol import test.support @@ -247,7 +246,6 @@ def test_parameter_repr(self): @test.support.cpython_only def test_from_param_result_refcount(self): # Issue #99952 - import sys import _ctypes_test from ctypes import PyDLL, c_int, c_void_p, py_object, Structure From a0b6672656a414e9f6a453f3e28b7c2e3947ab20 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith [Google LLC]" Date: Wed, 25 Jan 2023 23:54:50 -0800 Subject: [PATCH 5/6] add some comments. --- Lib/test/test_ctypes/test_parameters.py | 2 ++ Modules/_ctypes/_ctypes.c | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_ctypes/test_parameters.py b/Lib/test/test_ctypes/test_parameters.py index 629494bea58276..06cc95107b79fa 100644 --- a/Lib/test/test_ctypes/test_parameters.py +++ b/Lib/test/test_ctypes/test_parameters.py @@ -273,6 +273,7 @@ def test_from_param_result_refcount(self): from ctypes import PyDLL, c_int, c_void_p, py_object, Structure class X(Structure): + """This struct size is <= sizeof(void*).""" _fields_ = [("a", c_void_p)] def __del__(self): @@ -295,6 +296,7 @@ def from_param(cls, value): self.assertEqual(trace, [1, 2, 3, 4, 5]) class Y(Structure): + """This struct size is > sizeof(void*).""" _fields_ = [("a", c_void_p), ("b", c_void_p)] def __del__(self): diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index 87c4999d51dec2..8690f2c1b07852 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -412,7 +412,7 @@ _ctypes_alloc_format_string_with_shape(int ndim, const Py_ssize_t *shape, typedef struct { PyObject_HEAD void *ptr; - PyObject *keep; + PyObject *keep; // If set, a reference to the original CDataObject. } StructParamObject; From cf33e7c1dd8b43cf9e5ca1d0d178db5b28ce104a Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Wed, 25 Jan 2023 23:58:15 -0800 Subject: [PATCH 6/6] ReSTify the NEWS entry. --- .../next/Library/2022-12-11-14-38-59.gh-issue-99952.IYGLzr.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2022-12-11-14-38-59.gh-issue-99952.IYGLzr.rst b/Misc/NEWS.d/next/Library/2022-12-11-14-38-59.gh-issue-99952.IYGLzr.rst index b05be0c8f02799..09ec961249534f 100644 --- a/Misc/NEWS.d/next/Library/2022-12-11-14-38-59.gh-issue-99952.IYGLzr.rst +++ b/Misc/NEWS.d/next/Library/2022-12-11-14-38-59.gh-issue-99952.IYGLzr.rst @@ -1 +1,2 @@ -[ctypes] fix refcount issues in from_param() result. +Fix a reference undercounting issue in :class:`ctypes.Structure` with ``from_param()`` +results larger than a C pointer.