| |
@@ -1,246 +0,0 @@
|
| |
- From 091ae930cd8bac805a083ea68e4f886200f57de0 Mon Sep 17 00:00:00 2001
|
| |
- From: "Miss Islington (bot)"
|
| |
- <31488909+miss-islington@users.noreply.github.com>
|
| |
- Date: Fri, 30 Aug 2019 05:50:44 -0700
|
| |
- Subject: [PATCH] 00331: Fix StructUnionType_paramfunc()
|
| |
-
|
| |
- Fix a ctypes regression of Python 3.8. When a ctypes.Structure is
|
| |
- passed by copy to a function, ctypes internals created a temporary
|
| |
- object which had the side effect of calling the structure finalizer
|
| |
- (__del__) twice. The Python semantics requires a finalizer to be
|
| |
- called exactly once. Fix ctypes internals to no longer call the
|
| |
- finalizer twice.
|
| |
-
|
| |
- Create a new internal StructParam_Type which is only used by
|
| |
- _ctypes_callproc() to call PyMem_Free(ptr) on Py_DECREF(argument).
|
| |
- StructUnionType_paramfunc() creates such object.
|
| |
- (cherry picked from commit 96b4087ce784ee7434dffdf69c475f5b40543982)
|
| |
-
|
| |
- Co-authored-by: Victor Stinner <vstinner@redhat.com>
|
| |
- ---
|
| |
- Lib/ctypes/test/test_structures.py | 51 +++++++++++--
|
| |
- .../2019-08-30-11-21-10.bpo-37140.cFAX-a.rst | 5 ++
|
| |
- Modules/_ctypes/_ctypes.c | 73 +++++++++++++++----
|
| |
- 3 files changed, 109 insertions(+), 20 deletions(-)
|
| |
- create mode 100644 Misc/NEWS.d/next/Library/2019-08-30-11-21-10.bpo-37140.cFAX-a.rst
|
| |
-
|
| |
- diff --git a/Lib/ctypes/test/test_structures.py b/Lib/ctypes/test/test_structures.py
|
| |
- index d1ea43bc7e..fda104563d 100644
|
| |
- --- a/Lib/ctypes/test/test_structures.py
|
| |
- +++ b/Lib/ctypes/test/test_structures.py
|
| |
- @@ -3,7 +3,7 @@ from ctypes import *
|
| |
- from ctypes.test import need_symbol
|
| |
- from struct import calcsize
|
| |
- import _ctypes_test
|
| |
- -import test.support
|
| |
- +from test import support
|
| |
-
|
| |
- class SubclassesTest(unittest.TestCase):
|
| |
- def test_subclass(self):
|
| |
- @@ -202,7 +202,7 @@ class StructureTestCase(unittest.TestCase):
|
| |
- "_pack_": -1}
|
| |
- self.assertRaises(ValueError, type(Structure), "X", (Structure,), d)
|
| |
-
|
| |
- - @test.support.cpython_only
|
| |
- + @support.cpython_only
|
| |
- def test_packed_c_limits(self):
|
| |
- # Issue 15989
|
| |
- import _testcapi
|
| |
- @@ -396,27 +396,66 @@ class StructureTestCase(unittest.TestCase):
|
| |
- self.assertRaises(TypeError, lambda: Z(1, 2, 3, 4, 5, 6, 7))
|
| |
-
|
| |
- def test_pass_by_value(self):
|
| |
- - # This should mirror the structure in Modules/_ctypes/_ctypes_test.c
|
| |
- - class X(Structure):
|
| |
- + # This should mirror the Test structure
|
| |
- + # in Modules/_ctypes/_ctypes_test.c
|
| |
- + class Test(Structure):
|
| |
- _fields_ = [
|
| |
- ('first', c_ulong),
|
| |
- ('second', c_ulong),
|
| |
- ('third', c_ulong),
|
| |
- ]
|
| |
-
|
| |
- - s = X()
|
| |
- + s = Test()
|
| |
- s.first = 0xdeadbeef
|
| |
- s.second = 0xcafebabe
|
| |
- s.third = 0x0bad1dea
|
| |
- dll = CDLL(_ctypes_test.__file__)
|
| |
- func = dll._testfunc_large_struct_update_value
|
| |
- - func.argtypes = (X,)
|
| |
- + func.argtypes = (Test,)
|
| |
- func.restype = None
|
| |
- func(s)
|
| |
- self.assertEqual(s.first, 0xdeadbeef)
|
| |
- self.assertEqual(s.second, 0xcafebabe)
|
| |
- self.assertEqual(s.third, 0x0bad1dea)
|
| |
-
|
| |
- + def test_pass_by_value_finalizer(self):
|
| |
- + # bpo-37140: Similar to test_pass_by_value(), but the Python structure
|
| |
- + # has a finalizer (__del__() method): the finalizer must only be called
|
| |
- + # once.
|
| |
- +
|
| |
- + finalizer_calls = []
|
| |
- +
|
| |
- + class Test(Structure):
|
| |
- + _fields_ = [
|
| |
- + ('first', c_ulong),
|
| |
- + ('second', c_ulong),
|
| |
- + ('third', c_ulong),
|
| |
- + ]
|
| |
- + def __del__(self):
|
| |
- + finalizer_calls.append("called")
|
| |
- +
|
| |
- + s = Test(1, 2, 3)
|
| |
- + # Test the StructUnionType_paramfunc() code path which copies the
|
| |
- + # structure: if the stucture is larger than sizeof(void*).
|
| |
- + self.assertGreater(sizeof(s), sizeof(c_void_p))
|
| |
- +
|
| |
- + dll = CDLL(_ctypes_test.__file__)
|
| |
- + func = dll._testfunc_large_struct_update_value
|
| |
- + func.argtypes = (Test,)
|
| |
- + func.restype = None
|
| |
- + func(s)
|
| |
- + # bpo-37140: Passing the structure by refrence must not call
|
| |
- + # its finalizer!
|
| |
- + self.assertEqual(finalizer_calls, [])
|
| |
- + self.assertEqual(s.first, 1)
|
| |
- + self.assertEqual(s.second, 2)
|
| |
- + self.assertEqual(s.third, 3)
|
| |
- +
|
| |
- + # The finalizer must be called exactly once
|
| |
- + s = None
|
| |
- + support.gc_collect()
|
| |
- + self.assertEqual(finalizer_calls, ["called"])
|
| |
- +
|
| |
- def test_pass_by_value_in_register(self):
|
| |
- class X(Structure):
|
| |
- _fields_ = [
|
| |
- diff --git a/Misc/NEWS.d/next/Library/2019-08-30-11-21-10.bpo-37140.cFAX-a.rst b/Misc/NEWS.d/next/Library/2019-08-30-11-21-10.bpo-37140.cFAX-a.rst
|
| |
- new file mode 100644
|
| |
- index 0000000000..4eaa226147
|
| |
- --- /dev/null
|
| |
- +++ b/Misc/NEWS.d/next/Library/2019-08-30-11-21-10.bpo-37140.cFAX-a.rst
|
| |
- @@ -0,0 +1,5 @@
|
| |
- +Fix a ctypes regression of Python 3.8. When a ctypes.Structure is passed by
|
| |
- +copy to a function, ctypes internals created a temporary object which had
|
| |
- +the side effect of calling the structure finalizer (__del__) twice. The
|
| |
- +Python semantics requires a finalizer to be called exactly once. Fix ctypes
|
| |
- +internals to no longer call the finalizer twice.
|
| |
- diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c
|
| |
- index 2201c4520a..4728874006 100644
|
| |
- --- a/Modules/_ctypes/_ctypes.c
|
| |
- +++ b/Modules/_ctypes/_ctypes.c
|
| |
- @@ -392,6 +392,35 @@ _ctypes_alloc_format_string_with_shape(int ndim, const Py_ssize_t *shape,
|
| |
- return result;
|
| |
- }
|
| |
-
|
| |
- +/* StructParamObject and StructParam_Type are used in _ctypes_callproc()
|
| |
- + for argument.keep to call PyMem_Free(ptr) on Py_DECREF(argument).
|
| |
- +
|
| |
- + StructUnionType_paramfunc() creates such object when a ctypes Structure is
|
| |
- + passed by copy to a C function. */
|
| |
- +typedef struct {
|
| |
- + PyObject_HEAD
|
| |
- + void *ptr;
|
| |
- +} StructParamObject;
|
| |
- +
|
| |
- +
|
| |
- +static void
|
| |
- +StructParam_dealloc(PyObject *myself)
|
| |
- +{
|
| |
- + StructParamObject *self = (StructParamObject *)myself;
|
| |
- + PyMem_Free(self->ptr);
|
| |
- + Py_TYPE(self)->tp_free(myself);
|
| |
- +}
|
| |
- +
|
| |
- +
|
| |
- +static PyTypeObject StructParam_Type = {
|
| |
- + PyVarObject_HEAD_INIT(NULL, 0)
|
| |
- + .tp_name = "_ctypes.StructParam_Type",
|
| |
- + .tp_basicsize = sizeof(StructParamObject),
|
| |
- + .tp_dealloc = StructParam_dealloc,
|
| |
- + .tp_flags = Py_TPFLAGS_DEFAULT,
|
| |
- +};
|
| |
- +
|
| |
- +
|
| |
- /*
|
| |
- PyCStructType_Type - a meta type/class. Creating a new class using this one as
|
| |
- __metaclass__ will call the constructor StructUnionType_new. It replaces the
|
| |
- @@ -403,35 +432,47 @@ static PyCArgObject *
|
| |
- StructUnionType_paramfunc(CDataObject *self)
|
| |
- {
|
| |
- PyCArgObject *parg;
|
| |
- - CDataObject *copied_self;
|
| |
- + PyObject *obj;
|
| |
- StgDictObject *stgdict;
|
| |
- + void *ptr;
|
| |
-
|
| |
- if ((size_t)self->b_size > sizeof(void*)) {
|
| |
- - void *new_ptr = PyMem_Malloc(self->b_size);
|
| |
- - if (new_ptr == NULL)
|
| |
- + ptr = PyMem_Malloc(self->b_size);
|
| |
- + if (ptr == NULL) {
|
| |
- return NULL;
|
| |
- - memcpy(new_ptr, self->b_ptr, self->b_size);
|
| |
- - copied_self = (CDataObject *)PyCData_AtAddress(
|
| |
- - (PyObject *)Py_TYPE(self), new_ptr);
|
| |
- - copied_self->b_needsfree = 1;
|
| |
- + }
|
| |
- + memcpy(ptr, self->b_ptr, self->b_size);
|
| |
- +
|
| |
- + /* Create a Python object which calls PyMem_Free(ptr) in
|
| |
- + its deallocator. The object will be destroyed
|
| |
- + at _ctypes_callproc() cleanup. */
|
| |
- + obj = (&StructParam_Type)->tp_alloc(&StructParam_Type, 0);
|
| |
- + if (obj == NULL) {
|
| |
- + PyMem_Free(ptr);
|
| |
- + return NULL;
|
| |
- + }
|
| |
- +
|
| |
- + StructParamObject *struct_param = (StructParamObject *)obj;
|
| |
- + struct_param->ptr = ptr;
|
| |
- } else {
|
| |
- - copied_self = self;
|
| |
- - Py_INCREF(copied_self);
|
| |
- + ptr = self->b_ptr;
|
| |
- + obj = (PyObject *)self;
|
| |
- + Py_INCREF(obj);
|
| |
- }
|
| |
-
|
| |
- parg = PyCArgObject_new();
|
| |
- if (parg == NULL) {
|
| |
- - Py_DECREF(copied_self);
|
| |
- + Py_DECREF(obj);
|
| |
- return NULL;
|
| |
- }
|
| |
-
|
| |
- parg->tag = 'V';
|
| |
- - stgdict = PyObject_stgdict((PyObject *)copied_self);
|
| |
- + stgdict = PyObject_stgdict((PyObject *)self);
|
| |
- assert(stgdict); /* Cannot be NULL for structure/union instances */
|
| |
- parg->pffi_type = &stgdict->ffi_type_pointer;
|
| |
- - parg->value.p = copied_self->b_ptr;
|
| |
- - parg->size = copied_self->b_size;
|
| |
- - parg->obj = (PyObject *)copied_self;
|
| |
- + parg->value.p = ptr;
|
| |
- + parg->size = self->b_size;
|
| |
- + parg->obj = obj;
|
| |
- return parg;
|
| |
- }
|
| |
-
|
| |
- @@ -5700,6 +5741,10 @@ PyInit__ctypes(void)
|
| |
- if (PyType_Ready(&DictRemover_Type) < 0)
|
| |
- return NULL;
|
| |
-
|
| |
- + if (PyType_Ready(&StructParam_Type) < 0) {
|
| |
- + return NULL;
|
| |
- + }
|
| |
- +
|
| |
- #ifdef MS_WIN32
|
| |
- if (create_comerror() < 0)
|
| |
- return NULL;
|
| |
- --
|
| |
- 2.21.0
|
| |
-
|
| |
churchyard
commented 4 years ago
Rebased patches: 102
Removed patches already merged upstream: 331