Content-Length: 57575 | pFad | http://github.com/python/cpython/pull/128490.patch

thub.com From 3bb21fff034fb071124b78eb9b45c0d32783d3f9 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sat, 4 Jan 2025 11:24:55 -0500 Subject: [PATCH 01/24] Lock pointer reads and writes in ctypes. --- Modules/_ctypes/_ctypes.c | 117 +++++++++++++++++++++++++++----------- Modules/_ctypes/ctypes.h | 47 +++++++++++++++ 2 files changed, 132 insertions(+), 32 deletions(-) diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index ac520ffaad6c90..4a8eb218fc9554 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -598,7 +598,7 @@ StructUnionType_paramfunc(ctypes_state *st, CDataObject *self) if (ptr == NULL) { return NULL; } - memcpy(ptr, self->b_ptr, self->b_size); + locked_memcpy_from(ptr, self, self->b_size); /* Create a Python object which calls PyMem_Free(ptr) in its deallocator. The object will be destroyed @@ -907,8 +907,7 @@ CDataType_from_buffer_copy_impl(PyObject *type, PyTypeObject *cls, result = generic_pycdata_new(st, (PyTypeObject *)type, NULL, NULL); if (result != NULL) { - memcpy(((CDataObject *)result)->b_ptr, - (char *)buffer->buf + offset, info->size); + locked_memcpy_to((CDataObject *) result, buffer->buf + offset, info->size); } return result; } @@ -1195,7 +1194,7 @@ PyCPointerType_paramfunc(ctypes_state *st, CDataObject *self) parg->tag = 'P'; parg->pffi_type = &ffi_type_pointer; parg->obj = Py_NewRef(self); - parg->value.p = *(void **)self->b_ptr; + parg->value.p = locked_deref(self); return parg; } @@ -1432,7 +1431,7 @@ CharArray_set_raw(CDataObject *self, PyObject *value, void *Py_UNUSED(ignored)) goto fail; } - memcpy(self->b_ptr, ptr, size); + locked_memcpy_to(self, ptr, size); PyBuffer_Release(&view); return 0; @@ -1444,18 +1443,25 @@ CharArray_set_raw(CDataObject *self, PyObject *value, void *Py_UNUSED(ignored)) static PyObject * CharArray_get_raw(CDataObject *self, void *Py_UNUSED(ignored)) { - return PyBytes_FromStringAndSize(self->b_ptr, self->b_size); + LOCK_PTR(self); + // XXX Is it possible that PyBytes_FromStringAndSize is re-entrant? + PyObject *res = PyBytes_FromStringAndSize(self->b_ptr, self->b_size); + UNLOCK_PTR(self); + return res; } static PyObject * CharArray_get_value(CDataObject *self, void *Py_UNUSED(ignored)) { Py_ssize_t i; + LOCK_PTR(self); char *ptr = self->b_ptr; for (i = 0; i < self->b_size; ++i) if (*ptr++ == '\0') break; - return PyBytes_FromStringAndSize(self->b_ptr, i); + PyObject *res = PyBytes_FromStringAndSize(self->b_ptr, i); + UNLOCK_PTR(self); + return res; } static int @@ -1486,9 +1492,11 @@ CharArray_set_value(CDataObject *self, PyObject *value, void *Py_UNUSED(ignored) } ptr = PyBytes_AS_STRING(value); + LOCK_PTR(self); memcpy(self->b_ptr, ptr, size); if (size < self->b_size) self->b_ptr[size] = '\0'; + UNLOCK_PTR(self); Py_DECREF(value); return 0; @@ -1507,10 +1515,13 @@ WCharArray_get_value(CDataObject *self, void *Py_UNUSED(ignored)) { Py_ssize_t i; wchar_t *ptr = (wchar_t *)self->b_ptr; + LOCK_PTR(self); for (i = 0; i < self->b_size/(Py_ssize_t)sizeof(wchar_t); ++i) if (*ptr++ == (wchar_t)0) break; - return PyUnicode_FromWideChar((wchar_t *)self->b_ptr, i); + PyObject *res = PyUnicode_FromWideChar((wchar_t *)self->b_ptr, i); + UNLOCK_PTR(self); + return res; } static int @@ -1540,9 +1551,12 @@ WCharArray_set_value(CDataObject *self, PyObject *value, void *Py_UNUSED(ignored PyErr_SetString(PyExc_ValueError, "string too long"); return -1; } + LOCK_PTR(self); if (PyUnicode_AsWideChar(value, (wchar_t *)self->b_ptr, size) < 0) { + UNLOCK_PTR(self); return -1; } + UNLOCK_PTR(self); return 0; } @@ -2053,6 +2067,7 @@ c_void_p_from_param_impl(PyObject *type, PyTypeObject *cls, PyObject *value) parg->pffi_type = &ffi_type_pointer; parg->tag = 'P'; Py_INCREF(value); + // Function pointers don't change their contents, no need to lock parg->value.p = *(void **)func->b_ptr; parg->obj = value; return (PyObject *)parg; @@ -2079,7 +2094,7 @@ c_void_p_from_param_impl(PyObject *type, PyTypeObject *cls, PyObject *value) parg->tag = 'Z'; parg->obj = Py_NewRef(value); /* Remember: b_ptr points to where the pointer is stored! */ - parg->value.p = *(void **)(((CDataObject *)value)->b_ptr); + parg->value.p = locked_deref((CDataObject *)value); return (PyObject *)parg; } } @@ -2196,7 +2211,7 @@ PyCSimpleType_paramfunc(ctypes_state *st, CDataObject *self) parg->tag = fmt[0]; parg->pffi_type = fd->pffi_type; parg->obj = Py_NewRef(self); - memcpy(&parg->value, self->b_ptr, self->b_size); + locked_memcpy_from(&parg->value, self, self->b_size); return parg; } @@ -2697,7 +2712,7 @@ PyCFuncPtrType_paramfunc(ctypes_state *st, CDataObject *self) parg->tag = 'P'; parg->pffi_type = &ffi_type_pointer; parg->obj = Py_NewRef(self); - parg->value.p = *(void **)self->b_ptr; + parg->value.p = locked_deref(self); return parg; } @@ -3017,8 +3032,11 @@ PyCData_reduce_impl(PyObject *myself, PyTypeObject *cls) if (dict == NULL) { return NULL; } + LOCK_PTR(self); + PyObject *bytes = PyBytes_FromStringAndSize(self->b_ptr, self->b_size); + UNLOCK_PTR(self); return Py_BuildValue("O(O(NN))", st->_unpickle, Py_TYPE(myself), dict, - PyBytes_FromStringAndSize(self->b_ptr, self->b_size)); + bytes); } static PyObject * @@ -3036,7 +3054,10 @@ PyCData_setstate(PyObject *myself, PyObject *args) } if (len > self->b_size) len = self->b_size; + // XXX Can we use locked_memcpy_to()? + LOCK_PTR(self); memmove(self->b_ptr, data, len); + UNLOCK_PTR(self); mydict = PyObject_GetAttrString(myself, "__dict__"); if (mydict == NULL) { return NULL; @@ -3094,6 +3115,7 @@ static PyType_Spec pycdata_spec = { static int PyCData_MallocBuffer(CDataObject *obj, StgInfo *info) { + // We don't have to lock in this function if ((size_t)info->size <= sizeof(obj->b_value)) { /* No need to call malloc, can use the default buffer */ obj->b_ptr = (char *)&obj->b_value; @@ -3194,6 +3216,7 @@ PyCData_AtAddress(ctypes_state *st, PyObject *type, void *buf) return NULL; } assert(CDataObject_Check(st, pd)); + // XXX Use an atomic load if the size is 1, 2, 4, or 8 bytes? pd->b_ptr = (char *)buf; pd->b_length = info->length; pd->b_size = info->size; @@ -3288,9 +3311,7 @@ _PyCData_set(ctypes_state *st, if (err == -1) return NULL; if (err) { - memcpy(ptr, - src->b_ptr, - size); + locked_memcpy_from(ptr, src, size); if (PyCPointerTypeObject_Check(st, type)) { /* XXX */ @@ -3891,6 +3912,7 @@ PyCFuncPtr_FromDll(PyTypeObject *type, PyObject *args, PyObject *kwds) self->paramflags = Py_XNewRef(paramflags); + // No other threads can have this object *(void **)self->b_ptr = address; Py_INCREF(dll); Py_DECREF(ftuple); @@ -4823,18 +4845,23 @@ Array_subscript(PyObject *myself, PyObject *item) if (slicelen <= 0) return Py_GetConstant(Py_CONSTANT_EMPTY_BYTES); if (step == 1) { - return PyBytes_FromStringAndSize(ptr + start, + LOCK_PTR(self); + PyObject *res = PyBytes_FromStringAndSize(ptr + start, slicelen); + UNLOCK_PTR(self); + return res; } dest = (char *)PyMem_Malloc(slicelen); if (dest == NULL) return PyErr_NoMemory(); + LOCK_PTR(self); for (cur = start, i = 0; i < slicelen; cur += step, i++) { dest[i] = ptr[cur]; } + UNLOCK_PTR(self); np = PyBytes_FromStringAndSize(dest, slicelen); PyMem_Free(dest); @@ -4847,8 +4874,11 @@ Array_subscript(PyObject *myself, PyObject *item) if (slicelen <= 0) return Py_GetConstant(Py_CONSTANT_EMPTY_STR); if (step == 1) { - return PyUnicode_FromWideChar(ptr + start, + LOCK_PTR(self); + PyObject *res = PyUnicode_FromWideChar(ptr + start, slicelen); + UNLOCK_PTR(self); + return res; } dest = PyMem_New(wchar_t, slicelen); @@ -4857,10 +4887,12 @@ Array_subscript(PyObject *myself, PyObject *item) return NULL; } + LOCK_PTR(self); for (cur = start, i = 0; i < slicelen; cur += step, i++) { dest[i] = ptr[cur]; } + UNLOCK_PTR(self); np = PyUnicode_FromWideChar(dest, slicelen); PyMem_Free(dest); @@ -5118,7 +5150,9 @@ Simple_set_value(CDataObject *self, PyObject *value, void *Py_UNUSED(ignored)) assert(info); /* Cannot be NULL for CDataObject instances */ assert(info->setfunc); + LOCK_PTR(self); result = info->setfunc(self->b_ptr, value, info->size); + UNLOCK_PTR(self); if (!result) return -1; @@ -5147,7 +5181,10 @@ Simple_get_value(CDataObject *self, void *Py_UNUSED(ignored)) } assert(info); /* Cannot be NULL for CDataObject instances */ assert(info->getfunc); - return info->getfunc(self->b_ptr, self->b_size); + LOCK_PTR(self); + PyObject *res = info->getfunc(self->b_ptr, self->b_size); + UNLOCK_PTR(self); + return res; } static PyGetSetDef Simple_getsets[] = { @@ -5183,7 +5220,10 @@ static PyMethodDef Simple_methods[] = { static int Simple_bool(CDataObject *self) { - return memcmp(self->b_ptr, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0", self->b_size); + LOCK_PTR(self); + int cmp = memcmp(self->b_ptr, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0", self->b_size); + UNLOCK_PTR(self); + return cmp; } /* "%s(%s)" % (self.__class__.__name__, self.value) */ @@ -5239,8 +5279,9 @@ Pointer_item(PyObject *myself, Py_ssize_t index) Py_ssize_t size; Py_ssize_t offset; PyObject *proto; + void *deref = locked_deref(self); - if (*(void **)self->b_ptr == NULL) { + if (deref == NULL) { PyErr_SetString(PyExc_ValueError, "NULL pointer access"); return NULL; @@ -5267,7 +5308,7 @@ Pointer_item(PyObject *myself, Py_ssize_t index) offset = index * iteminfo->size; return PyCData_get(st, proto, stginfo->getfunc, (PyObject *)self, - index, size, (*(char **)self->b_ptr) + offset); + index, size, (deref + offset)); } static int @@ -5284,7 +5325,8 @@ Pointer_ass_item(PyObject *myself, Py_ssize_t index, PyObject *value) return -1; } - if (*(void **)self->b_ptr == NULL) { + void *deref = locked_deref(self); + if (deref == NULL) { PyErr_SetString(PyExc_ValueError, "NULL pointer access"); return -1; @@ -5311,13 +5353,14 @@ Pointer_ass_item(PyObject *myself, Py_ssize_t index, PyObject *value) offset = index * iteminfo->size; return PyCData_set(st, (PyObject *)self, proto, stginfo->setfunc, value, - index, size, (*(char **)self->b_ptr) + offset); + index, size, (deref + offset)); } static PyObject * Pointer_get_contents(CDataObject *self, void *closure) { - if (*(void **)self->b_ptr == NULL) { + void *deref = locked_deref(self); + if (deref == NULL) { PyErr_SetString(PyExc_ValueError, "NULL pointer access"); return NULL; @@ -5332,7 +5375,7 @@ Pointer_get_contents(CDataObject *self, void *closure) return PyCData_FromBaseObj(st, stginfo->proto, (PyObject *)self, 0, - *(void **)self->b_ptr); + deref); } static int @@ -5367,7 +5410,7 @@ Pointer_set_contents(CDataObject *self, PyObject *value, void *closure) } dst = (CDataObject *)value; - *(void **)self->b_ptr = dst->b_ptr; + locked_deref_assign(self, dst->b_ptr); /* A Pointer instance must keep the value it points to alive. So, a @@ -5502,41 +5545,51 @@ Pointer_subscript(PyObject *myself, PyObject *item) } assert(iteminfo); if (iteminfo->getfunc == _ctypes_get_fielddesc("c")->getfunc) { - char *ptr = *(char **)self->b_ptr; + char *ptr = locked_deref(self); char *dest; if (len <= 0) return Py_GetConstant(Py_CONSTANT_EMPTY_BYTES); if (step == 1) { - return PyBytes_FromStringAndSize(ptr + start, + LOCK_PTR(self); + PyObject *res = PyBytes_FromStringAndSize(ptr + start, len); + UNLOCK_PTR(self); + return res; } dest = (char *)PyMem_Malloc(len); if (dest == NULL) return PyErr_NoMemory(); + LOCK_PTR(self); for (cur = start, i = 0; i < len; cur += step, i++) { dest[i] = ptr[cur]; } + UNLOCK_PTR(self); np = PyBytes_FromStringAndSize(dest, len); PyMem_Free(dest); return np; } if (iteminfo->getfunc == _ctypes_get_fielddesc("u")->getfunc) { - wchar_t *ptr = *(wchar_t **)self->b_ptr; + wchar_t *ptr = locked_deref(self); wchar_t *dest; if (len <= 0) return Py_GetConstant(Py_CONSTANT_EMPTY_STR); if (step == 1) { - return PyUnicode_FromWideChar(ptr + start, + LOCK_PTR(self); + PyObject *res = PyUnicode_FromWideChar(ptr + start, len); + UNLOCK_PTR(self); + return res; } dest = PyMem_New(wchar_t, len); if (dest == NULL) return PyErr_NoMemory(); + LOCK_PTR(self); for (cur = start, i = 0; i < len; cur += step, i++) { dest[i] = ptr[cur]; } + UNLOCK_PTR(self); np = PyUnicode_FromWideChar(dest, len); PyMem_Free(dest); return np; @@ -5562,7 +5615,7 @@ Pointer_subscript(PyObject *myself, PyObject *item) static int Pointer_bool(CDataObject *self) { - return (*(void **)self->b_ptr != NULL); + return locked_deref(self) != NULL; } static PyType_Slot pycpointer_slots[] = { @@ -5770,7 +5823,7 @@ cast(void *ptr, PyObject *src, PyObject *ctype) } } /* Should we assert that result is a pointer type? */ - memcpy(result->b_ptr, &ptr, sizeof(void *)); + locked_memcpy_to(result, &ptr, sizeof(void *)); return (PyObject *)result; failed: diff --git a/Modules/_ctypes/ctypes.h b/Modules/_ctypes/ctypes.h index 45e00a538fb5a5..ce4acdfef820c6 100644 --- a/Modules/_ctypes/ctypes.h +++ b/Modules/_ctypes/ctypes.h @@ -152,6 +152,9 @@ union value { struct tagCDataObject { PyObject_HEAD +#ifdef Py_GIL_DISABLED + PyMutex b_ptr_lock; +#endif char *b_ptr; /* pointer to memory block */ int b_needsfree; /* need _we_ free the memory? */ CDataObject *b_base; /* pointer to base object or NULL */ @@ -181,6 +184,9 @@ typedef struct { typedef struct { /* First part identical to tagCDataObject */ PyObject_HEAD +#ifdef Py_GIL_DISABLED + PyMutex b_ptr_lock; +#endif char *b_ptr; /* pointer to memory block */ int b_needsfree; /* need _we_ free the memory? */ CDataObject *b_base; /* pointer to base object or NULL */ @@ -543,3 +549,44 @@ PyStgInfo_Init(ctypes_state *state, PyTypeObject *type) info->initialized = 1; return info; } + +#ifdef Py_GIL_DISABLED +# define LOCK_PTR(self) PyMutex_Lock(&self->b_ptr_lock) +# define UNLOCK_PTR(self) PyMutex_Unlock(&self->b_ptr_lock) +#else +# define LOCK_PTR(self) +# define UNLOCK_PTR(self) +#endif + +static inline void +locked_memcpy_to(CDataObject *self, void *buf, Py_ssize_t size) +{ + LOCK_PTR(self); + memcpy(self->b_ptr, buf, size); + UNLOCK_PTR(self); +} + +static inline void +locked_memcpy_from(void *buf, CDataObject *self, Py_ssize_t size) +{ + LOCK_PTR(self); + memcpy(buf, self->b_ptr, size); + UNLOCK_PTR(self); +} + +static inline void * +locked_deref(CDataObject *self) +{ + LOCK_PTR(self); + void *ptr = *(void **)self->b_ptr; + UNLOCK_PTR(self); + return ptr; +} + +static inline void +locked_deref_assign(CDataObject *self, void *new_ptr) +{ + LOCK_PTR(self); + *self->b_ptr = new_ptr; + UNLOCK_PTR(self); +} From 8c4c49a4c1862274b66c620fbbd72f8e807e42ef Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sat, 4 Jan 2025 11:29:32 -0500 Subject: [PATCH 02/24] Add a test. --- Lib/test/test_ctypes/test_arrays.py | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_ctypes/test_arrays.py b/Lib/test/test_ctypes/test_arrays.py index c80fdff5de685d..da61837db1fc3b 100644 --- a/Lib/test/test_ctypes/test_arrays.py +++ b/Lib/test/test_ctypes/test_arrays.py @@ -5,7 +5,7 @@ create_string_buffer, create_unicode_buffer, c_char, c_wchar, c_byte, c_ubyte, c_short, c_ushort, c_int, c_uint, c_long, c_ulonglong, c_float, c_double, c_longdouble) -from test.support import bigmemtest, _2G +from test.support import bigmemtest, _2G, threading_helper from ._support import (_CData, PyCArrayType, Py_TPFLAGS_DISALLOW_INSTANTIATION, Py_TPFLAGS_IMMUTABLETYPE) @@ -267,6 +267,24 @@ def test_bpo36504_signed_int_overflow(self): def test_large_array(self, size): c_char * size + @threading_helper.requires_working_threading() + @unittest.skipUnless(support.Py_GIL_DISABLED, "only meaningful if the GIL is disabled") + def test_thread_safety(self): + from threading import Thread + + buffer = (ctypes.c_char_p * 10)() + + def run(): + for i in range(100): + buffer.value = b"hello" + buffer[0] = b"j" + + with threading_helper.catch_threading_exception() as cm: + with threading_helper.start_threads((run for _ in range(25))): + pass + + self.assertIsNone(cm.exc_value) + if __name__ == '__main__': unittest.main() From da293c14262a946b00c08fa45aa3ea7dd2551fda Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sat, 4 Jan 2025 11:31:52 -0500 Subject: [PATCH 03/24] Fix the test. --- Lib/test/test_ctypes/test_arrays.py | 6 +++--- Modules/_ctypes/ctypes.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_ctypes/test_arrays.py b/Lib/test/test_ctypes/test_arrays.py index da61837db1fc3b..a575ebc76ff582 100644 --- a/Lib/test/test_ctypes/test_arrays.py +++ b/Lib/test/test_ctypes/test_arrays.py @@ -5,7 +5,7 @@ create_string_buffer, create_unicode_buffer, c_char, c_wchar, c_byte, c_ubyte, c_short, c_ushort, c_int, c_uint, c_long, c_ulonglong, c_float, c_double, c_longdouble) -from test.support import bigmemtest, _2G, threading_helper +from test.support import bigmemtest, _2G, threading_helper, Py_GIL_DISABLED from ._support import (_CData, PyCArrayType, Py_TPFLAGS_DISALLOW_INSTANTIATION, Py_TPFLAGS_IMMUTABLETYPE) @@ -268,7 +268,7 @@ def test_large_array(self, size): c_char * size @threading_helper.requires_working_threading() - @unittest.skipUnless(support.Py_GIL_DISABLED, "only meaningful if the GIL is disabled") + @unittest.skipUnless(Py_GIL_DISABLED, "only meaningful if the GIL is disabled") def test_thread_safety(self): from threading import Thread @@ -280,7 +280,7 @@ def run(): buffer[0] = b"j" with threading_helper.catch_threading_exception() as cm: - with threading_helper.start_threads((run for _ in range(25))): + with threading_helper.start_threads((Thread(target=run) for _ in range(25))): pass self.assertIsNone(cm.exc_value) diff --git a/Modules/_ctypes/ctypes.h b/Modules/_ctypes/ctypes.h index ce4acdfef820c6..77db066d62734c 100644 --- a/Modules/_ctypes/ctypes.h +++ b/Modules/_ctypes/ctypes.h @@ -587,6 +587,6 @@ static inline void locked_deref_assign(CDataObject *self, void *new_ptr) { LOCK_PTR(self); - *self->b_ptr = new_ptr; + *(void **)self->b_ptr = new_ptr; UNLOCK_PTR(self); } From d01b757f596d97987f64e73d413fdf551b7c43fd Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sat, 4 Jan 2025 11:32:52 -0500 Subject: [PATCH 04/24] Add blurb. --- .../next/Library/2025-01-04-11-32-46.gh-issue-128182.SJ2Zsa.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2025-01-04-11-32-46.gh-issue-128182.SJ2Zsa.rst diff --git a/Misc/NEWS.d/next/Library/2025-01-04-11-32-46.gh-issue-128182.SJ2Zsa.rst b/Misc/NEWS.d/next/Library/2025-01-04-11-32-46.gh-issue-128182.SJ2Zsa.rst new file mode 100644 index 00000000000000..038fecb5710436 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-01-04-11-32-46.gh-issue-128182.SJ2Zsa.rst @@ -0,0 +1,2 @@ +Fix crash when using :mod:`ctypes` pointers concurrently on the :term:`free +threaded ` build. From 8bb980f519854417ebb28c1550dd8c57e0b0f4a3 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sat, 4 Jan 2025 11:57:44 -0500 Subject: [PATCH 05/24] Add a lock for generic helper functions. --- Modules/_ctypes/_ctypes.c | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index 4a8eb218fc9554..7ec7241c1881fd 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -3216,7 +3216,6 @@ PyCData_AtAddress(ctypes_state *st, PyObject *type, void *buf) return NULL; } assert(CDataObject_Check(st, pd)); - // XXX Use an atomic load if the size is 1, 2, 4, or 8 bytes? pd->b_ptr = (char *)buf; pd->b_length = info->length; pd->b_size = info->size; @@ -3242,15 +3241,23 @@ PyObject * PyCData_get(ctypes_state *st, PyObject *type, GETFUNC getfunc, PyObject *src, Py_ssize_t index, Py_ssize_t size, char *adr) { - if (getfunc) - return getfunc(adr, size); + CDataObject *cdata = (CDataObject *)src; + if (getfunc) { + LOCK_PTR(cdata); + PyObject *res = getfunc(adr, size); + UNLOCK_PTR(cdata); + return res; + } assert(type); StgInfo *info; if (PyStgInfo_FromType(st, type, &info) < 0) { return NULL; } if (info && info->getfunc && !_ctypes_simple_instance(st, type)) { - return info->getfunc(adr, size); + LOCK_PTR(cdata); + PyObject *res = info->getfunc(adr, size); + UNLOCK_PTR(cdata); + return res; } return PyCData_FromBaseObj(st, type, src, index, adr); } @@ -3267,15 +3274,22 @@ _PyCData_set(ctypes_state *st, int err; if (setfunc) { - return setfunc(ptr, value, size); + LOCK_PTR(dst); + PyObject *res = setfunc(ptr, value, size); + UNLOCK_PTR(dst); + return res; } if (!CDataObject_Check(st, value)) { StgInfo *info; if (PyStgInfo_FromType(st, type, &info) < 0) { return NULL; } - if (info && info->setfunc) - return info->setfunc(ptr, value, size); + if (info && info->setfunc) { + LOCK_PTR(dst); + PyObject *res = info->setfunc(ptr, value, size); + UNLOCK_PTR(dst); + return res; + } /* If value is a tuple, we try to call the type with the tuple and use the result! @@ -3295,7 +3309,9 @@ _PyCData_set(ctypes_state *st, Py_DECREF(ob); return result; } else if (value == Py_None && PyCPointerTypeObject_Check(st, type)) { + LOCK_PTR(dst); *(void **)ptr = NULL; + UNLOCK_PTR(dst); Py_RETURN_NONE; } else { PyErr_Format(PyExc_TypeError, @@ -3345,7 +3361,9 @@ _PyCData_set(ctypes_state *st, ((PyTypeObject *)type)->tp_name); return NULL; } + LOCK_PTR(dst); *(void **)ptr = src->b_ptr; + UNLOCK_PTR(dst); keep = GetKeepedObjects(src); if (keep == NULL) From d60f2e237d4f8ee4265c873186e3dbfb6c93a29b Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sat, 4 Jan 2025 12:20:13 -0500 Subject: [PATCH 06/24] Add documentation note. --- Doc/library/ctypes.rst | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/Doc/library/ctypes.rst b/Doc/library/ctypes.rst index 09692e56d29a39..c0c2cf35d6a0a6 100644 --- a/Doc/library/ctypes.rst +++ b/Doc/library/ctypes.rst @@ -870,6 +870,35 @@ invalid non-\ ``NULL`` pointers would crash Python):: ValueError: NULL pointer access >>> +.. _ctypes-thread-safety: + +Thread Safety Without The GIL +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +In Python 3.13, the :term:`GIL` may be disabled on :term:`experimental free threaded ` builds. +In ctypes, reads and writes to a single object concurrently is safe, but not across multiple objects.:: + + >>> number = c_int(42) + >>> pointer_a = pointer(number) + >>> pointer_b = pointer(number) + +In the above, it's only safe for one object to read and write to the address at once if the :term:`GIL` is disabled. +So, ``pointer_a`` can be shared and written to across multiple threads, but only if ``pointer_b`` +is not also attempting to do the same. If this is an issue, consider using a :class:`threading.Lock` +to synchronize access to memory.:: + + >>> import threading + >>> lock = threading.Lock() + >>> # Thread 1 + >>> with lock: + ... pointer_a.contents = 24 + >>> # Thread 2 + >>> with lock: + ... pointer_b.contents = 42 + +.. seealso:: + + :func:`sys._is_gil_enabled` if you would like to dynamically synchronize your application. .. _ctypes-type-conversions: From de4c4eabe421b688cf3d50d10fd14f163500a28a Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sat, 4 Jan 2025 12:42:29 -0500 Subject: [PATCH 07/24] Fix compiler warning on GIL-ful builds. --- Modules/_ctypes/_ctypes.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index 7ec7241c1881fd..f5ac476789ed54 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -3241,7 +3241,10 @@ PyObject * PyCData_get(ctypes_state *st, PyObject *type, GETFUNC getfunc, PyObject *src, Py_ssize_t index, Py_ssize_t size, char *adr) { +#ifdef Py_GIL_DISABLED + // This isn't used if the GIL is enabled, so it causes a compiler warning. CDataObject *cdata = (CDataObject *)src; +#endif if (getfunc) { LOCK_PTR(cdata); PyObject *res = getfunc(adr, size); From 23d8e21f7c716d164a71dbe39dd1f51cc86a8bea Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sat, 4 Jan 2025 12:54:59 -0500 Subject: [PATCH 08/24] Fix MSVC compiler warnings. --- Modules/_ctypes/_ctypes.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index f5ac476789ed54..f9123ff5a96032 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -907,7 +907,7 @@ CDataType_from_buffer_copy_impl(PyObject *type, PyTypeObject *cls, result = generic_pycdata_new(st, (PyTypeObject *)type, NULL, NULL); if (result != NULL) { - locked_memcpy_to((CDataObject *) result, buffer->buf + offset, info->size); + locked_memcpy_to((CDataObject *) result, (char *)buffer->buf + offset, info->size); } return result; } @@ -5329,7 +5329,7 @@ Pointer_item(PyObject *myself, Py_ssize_t index) offset = index * iteminfo->size; return PyCData_get(st, proto, stginfo->getfunc, (PyObject *)self, - index, size, (deref + offset)); + index, size, (char *)(deref + offset)); } static int @@ -5374,7 +5374,7 @@ Pointer_ass_item(PyObject *myself, Py_ssize_t index, PyObject *value) offset = index * iteminfo->size; return PyCData_set(st, (PyObject *)self, proto, stginfo->setfunc, value, - index, size, (deref + offset)); + index, size, (char *)(deref + offset)); } static PyObject * From bdee2b253527dfd2a3407a4349d10ab0c545473b Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sat, 4 Jan 2025 15:47:55 -0500 Subject: [PATCH 09/24] Fix MSVC compiler warnings (actually this time) --- Modules/_ctypes/_ctypes.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index f9123ff5a96032..35837a1b91a47f 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -5329,7 +5329,7 @@ Pointer_item(PyObject *myself, Py_ssize_t index) offset = index * iteminfo->size; return PyCData_get(st, proto, stginfo->getfunc, (PyObject *)self, - index, size, (char *)(deref + offset)); + index, size, (char *)((char *)deref + offset)); } static int @@ -5374,7 +5374,7 @@ Pointer_ass_item(PyObject *myself, Py_ssize_t index, PyObject *value) offset = index * iteminfo->size; return PyCData_set(st, (PyObject *)self, proto, stginfo->setfunc, value, - index, size, (char *)(deref + offset)); + index, size, ((char *)deref + offset)); } static PyObject * From 9f272d94a848ab109322477e2a82cfeb5bd5ac2f Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Mon, 6 Jan 2025 09:51:33 -0500 Subject: [PATCH 10/24] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com> --- Doc/library/ctypes.rst | 14 ++++++++------ Lib/test/test_ctypes/test_arrays.py | 3 ++- Modules/_ctypes/ctypes.h | 4 ++-- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/Doc/library/ctypes.rst b/Doc/library/ctypes.rst index c0c2cf35d6a0a6..b78e65ab5f0b62 100644 --- a/Doc/library/ctypes.rst +++ b/Doc/library/ctypes.rst @@ -872,15 +872,17 @@ invalid non-\ ``NULL`` pointers would crash Python):: .. _ctypes-thread-safety: -Thread Safety Without The GIL +Thread safety without the GIL ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ In Python 3.13, the :term:`GIL` may be disabled on :term:`experimental free threaded ` builds. -In ctypes, reads and writes to a single object concurrently is safe, but not across multiple objects.:: +In ctypes, reads and writes to a single object concurrently is safe, but not across multiple objects: - >>> number = c_int(42) - >>> pointer_a = pointer(number) - >>> pointer_b = pointer(number) + .. code-block:: pycon + + >>> number = c_int(42) + >>> pointer_a = pointer(number) + >>> pointer_b = pointer(number) In the above, it's only safe for one object to read and write to the address at once if the :term:`GIL` is disabled. So, ``pointer_a`` can be shared and written to across multiple threads, but only if ``pointer_b`` @@ -898,7 +900,7 @@ to synchronize access to memory.:: .. seealso:: - :func:`sys._is_gil_enabled` if you would like to dynamically synchronize your application. + Use :func:`sys._is_gil_enabled` to dynamically synchronize your application. .. _ctypes-type-conversions: diff --git a/Lib/test/test_ctypes/test_arrays.py b/Lib/test/test_ctypes/test_arrays.py index a575ebc76ff582..fe76a7f382d49c 100644 --- a/Lib/test/test_ctypes/test_arrays.py +++ b/Lib/test/test_ctypes/test_arrays.py @@ -280,7 +280,8 @@ def run(): buffer[0] = b"j" with threading_helper.catch_threading_exception() as cm: - with threading_helper.start_threads((Thread(target=run) for _ in range(25))): + threads = (Thread(target=run) for _ in range(25)) + with threading_helper.start_threads(threads): pass self.assertIsNone(cm.exc_value) diff --git a/Modules/_ctypes/ctypes.h b/Modules/_ctypes/ctypes.h index 77db066d62734c..5e004403c4189a 100644 --- a/Modules/_ctypes/ctypes.h +++ b/Modules/_ctypes/ctypes.h @@ -562,7 +562,7 @@ static inline void locked_memcpy_to(CDataObject *self, void *buf, Py_ssize_t size) { LOCK_PTR(self); - memcpy(self->b_ptr, buf, size); + (void)memcpy(self->b_ptr, buf, size); UNLOCK_PTR(self); } @@ -570,7 +570,7 @@ static inline void locked_memcpy_from(void *buf, CDataObject *self, Py_ssize_t size) { LOCK_PTR(self); - memcpy(buf, self->b_ptr, size); + (void)memcpy(buf, self->b_ptr, size); UNLOCK_PTR(self); } From 32a885feee29c3a4b4397a50016d3c186478e119 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Mon, 6 Jan 2025 10:23:14 -0500 Subject: [PATCH 11/24] Fix Sphinx code block. --- Doc/library/ctypes.rst | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/Doc/library/ctypes.rst b/Doc/library/ctypes.rst index b78e65ab5f0b62..2a7977fcd9a0d3 100644 --- a/Doc/library/ctypes.rst +++ b/Doc/library/ctypes.rst @@ -887,16 +887,18 @@ In ctypes, reads and writes to a single object concurrently is safe, but not acr In the above, it's only safe for one object to read and write to the address at once if the :term:`GIL` is disabled. So, ``pointer_a`` can be shared and written to across multiple threads, but only if ``pointer_b`` is not also attempting to do the same. If this is an issue, consider using a :class:`threading.Lock` -to synchronize access to memory.:: - - >>> import threading - >>> lock = threading.Lock() - >>> # Thread 1 - >>> with lock: - ... pointer_a.contents = 24 - >>> # Thread 2 - >>> with lock: - ... pointer_b.contents = 42 +to synchronize access to memory: + + .. code-block:: pycon + + >>> import threading + >>> lock = threading.Lock() + >>> # Thread 1 + >>> with lock: + ... pointer_a.contents = 24 + >>> # Thread 2 + >>> with lock: + ... pointer_b.contents = 42 .. seealso:: From 1c92b4bacaabff63539ad09cb2f882f87075080e Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Mon, 6 Jan 2025 10:24:17 -0500 Subject: [PATCH 12/24] Flatten call to PyUnicode_AsWideChar --- Modules/_ctypes/_ctypes.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index 35837a1b91a47f..282971e42abe68 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -1552,12 +1552,9 @@ WCharArray_set_value(CDataObject *self, PyObject *value, void *Py_UNUSED(ignored return -1; } LOCK_PTR(self); - if (PyUnicode_AsWideChar(value, (wchar_t *)self->b_ptr, size) < 0) { - UNLOCK_PTR(self); - return -1; - } + int rc = PyUnicode_AsWideChar(value, (wchar_t *)self->b_ptr, size); UNLOCK_PTR(self); - return 0; + return rc; } static PyGetSetDef WCharArray_getsets[] = { From ff34cc2c49e26e18b31c7f4c0a934b33214f9c52 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Mon, 6 Jan 2025 10:26:34 -0500 Subject: [PATCH 13/24] Fix lock. --- Modules/_ctypes/_ctypes.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index 282971e42abe68..519ecca51ee3f1 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -3361,9 +3361,9 @@ _PyCData_set(ctypes_state *st, ((PyTypeObject *)type)->tp_name); return NULL; } - LOCK_PTR(dst); + LOCK_PTR(src); *(void **)ptr = src->b_ptr; - UNLOCK_PTR(dst); + UNLOCK_PTR(src); keep = GetKeepedObjects(src); if (keep == NULL) From 804f1b65304767059bf645a5b6e9bcb19f756e79 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Mon, 6 Jan 2025 10:28:33 -0500 Subject: [PATCH 14/24] Fix indentation. --- Modules/_ctypes/_ctypes.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index 519ecca51ee3f1..7739830e4f0b59 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -4865,7 +4865,7 @@ Array_subscript(PyObject *myself, PyObject *item) if (step == 1) { LOCK_PTR(self); PyObject *res = PyBytes_FromStringAndSize(ptr + start, - slicelen); + slicelen); UNLOCK_PTR(self); return res; } @@ -4894,7 +4894,7 @@ Array_subscript(PyObject *myself, PyObject *item) if (step == 1) { LOCK_PTR(self); PyObject *res = PyUnicode_FromWideChar(ptr + start, - slicelen); + slicelen); UNLOCK_PTR(self); return res; } @@ -5371,7 +5371,7 @@ Pointer_ass_item(PyObject *myself, Py_ssize_t index, PyObject *value) offset = index * iteminfo->size; return PyCData_set(st, (PyObject *)self, proto, stginfo->setfunc, value, - index, size, ((char *)deref + offset)); + index, size, ((char *)deref + offset)); } static PyObject * @@ -5571,7 +5571,7 @@ Pointer_subscript(PyObject *myself, PyObject *item) if (step == 1) { LOCK_PTR(self); PyObject *res = PyBytes_FromStringAndSize(ptr + start, - len); + len); UNLOCK_PTR(self); return res; } @@ -5596,7 +5596,7 @@ Pointer_subscript(PyObject *myself, PyObject *item) if (step == 1) { LOCK_PTR(self); PyObject *res = PyUnicode_FromWideChar(ptr + start, - len); + len); UNLOCK_PTR(self); return res; } From bae3c5bb27f54f077cf0d2afa5931fb1a7d86552 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Mon, 6 Jan 2025 10:41:20 -0500 Subject: [PATCH 15/24] Revert "Flatten call to PyUnicode_AsWideChar" This reverts commit 1c92b4bacaabff63539ad09cb2f882f87075080e. --- Modules/_ctypes/_ctypes.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index 7739830e4f0b59..b591097826e956 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -1552,9 +1552,12 @@ WCharArray_set_value(CDataObject *self, PyObject *value, void *Py_UNUSED(ignored return -1; } LOCK_PTR(self); - int rc = PyUnicode_AsWideChar(value, (wchar_t *)self->b_ptr, size); + if (PyUnicode_AsWideChar(value, (wchar_t *)self->b_ptr, size) < 0) { + UNLOCK_PTR(self); + return -1; + } UNLOCK_PTR(self); - return rc; + return 0; } static PyGetSetDef WCharArray_getsets[] = { From d720d9ab18dc506f2bb36fee3ea5b5145483159b Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Mon, 6 Jan 2025 10:42:14 -0500 Subject: [PATCH 16/24] Properly flatten call to PyUnicode_AsWideChar --- Modules/_ctypes/_ctypes.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index b591097826e956..da3df76f024d41 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -1552,12 +1552,9 @@ WCharArray_set_value(CDataObject *self, PyObject *value, void *Py_UNUSED(ignored return -1; } LOCK_PTR(self); - if (PyUnicode_AsWideChar(value, (wchar_t *)self->b_ptr, size) < 0) { - UNLOCK_PTR(self); - return -1; - } + int rc = PyUnicode_AsWideChar(value, (wchar_t *)self->b_ptr, size); UNLOCK_PTR(self); - return 0; + return rc < 0 ? -1 : 0; } static PyGetSetDef WCharArray_getsets[] = { From 04b32527c45cb1fb32d10fa175814606ad5f20e4 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Mon, 6 Jan 2025 10:43:19 -0500 Subject: [PATCH 17/24] Fix indentation. --- Modules/_ctypes/_ctypes.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index da3df76f024d41..91c3ce060d9525 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -5326,7 +5326,7 @@ Pointer_item(PyObject *myself, Py_ssize_t index) offset = index * iteminfo->size; return PyCData_get(st, proto, stginfo->getfunc, (PyObject *)self, - index, size, (char *)((char *)deref + offset)); + index, size, (char *)((char *)deref + offset)); } static int From 6401bbcecfc204ff2984a41e59122b5abb8efbca Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Mon, 6 Jan 2025 10:44:31 -0500 Subject: [PATCH 18/24] Clarify comment --- Modules/_ctypes/_ctypes.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index 91c3ce060d9525..b89590bddcd597 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -3112,7 +3112,9 @@ static PyType_Spec pycdata_spec = { static int PyCData_MallocBuffer(CDataObject *obj, StgInfo *info) { - // We don't have to lock in this function + /* We don't have to lock in this function, because it's only + used in constructors and therefore does not have concurrent + access. */ if ((size_t)info->size <= sizeof(obj->b_value)) { /* No need to call malloc, can use the default buffer */ obj->b_ptr = (char *)&obj->b_value; From 9d35e86f68fc8d4fbccb2660ea9899d875e5c8e8 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Wed, 8 Jan 2025 16:45:04 -0500 Subject: [PATCH 19/24] Switch to critical sections. --- Modules/_ctypes/_ctypes.c | 55 +++++++++++++++++++++++++-------------- Modules/_ctypes/ctypes.h | 15 +++++------ 2 files changed, 41 insertions(+), 29 deletions(-) diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index b89590bddcd597..138ef2d66f3416 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -1443,9 +1443,9 @@ CharArray_set_raw(CDataObject *self, PyObject *value, void *Py_UNUSED(ignored)) static PyObject * CharArray_get_raw(CDataObject *self, void *Py_UNUSED(ignored)) { + PyObject *res; LOCK_PTR(self); - // XXX Is it possible that PyBytes_FromStringAndSize is re-entrant? - PyObject *res = PyBytes_FromStringAndSize(self->b_ptr, self->b_size); + res = PyBytes_FromStringAndSize(self->b_ptr, self->b_size); UNLOCK_PTR(self); return res; } @@ -1454,12 +1454,13 @@ static PyObject * CharArray_get_value(CDataObject *self, void *Py_UNUSED(ignored)) { Py_ssize_t i; + PyObject *res; LOCK_PTR(self); char *ptr = self->b_ptr; for (i = 0; i < self->b_size; ++i) if (*ptr++ == '\0') break; - PyObject *res = PyBytes_FromStringAndSize(self->b_ptr, i); + res = PyBytes_FromStringAndSize(self->b_ptr, i); UNLOCK_PTR(self); return res; } @@ -1514,12 +1515,13 @@ static PyObject * WCharArray_get_value(CDataObject *self, void *Py_UNUSED(ignored)) { Py_ssize_t i; + PyObject *res; wchar_t *ptr = (wchar_t *)self->b_ptr; LOCK_PTR(self); for (i = 0; i < self->b_size/(Py_ssize_t)sizeof(wchar_t); ++i) if (*ptr++ == (wchar_t)0) break; - PyObject *res = PyUnicode_FromWideChar((wchar_t *)self->b_ptr, i); + res = PyUnicode_FromWideChar((wchar_t *)self->b_ptr, i); UNLOCK_PTR(self); return res; } @@ -1551,8 +1553,9 @@ WCharArray_set_value(CDataObject *self, PyObject *value, void *Py_UNUSED(ignored PyErr_SetString(PyExc_ValueError, "string too long"); return -1; } + int rc; LOCK_PTR(self); - int rc = PyUnicode_AsWideChar(value, (wchar_t *)self->b_ptr, size); + rc = PyUnicode_AsWideChar(value, (wchar_t *)self->b_ptr, size); UNLOCK_PTR(self); return rc < 0 ? -1 : 0; } @@ -3029,8 +3032,9 @@ PyCData_reduce_impl(PyObject *myself, PyTypeObject *cls) if (dict == NULL) { return NULL; } + PyObject *bytes; LOCK_PTR(self); - PyObject *bytes = PyBytes_FromStringAndSize(self->b_ptr, self->b_size); + bytes = PyBytes_FromStringAndSize(self->b_ptr, self->b_size); UNLOCK_PTR(self); return Py_BuildValue("O(O(NN))", st->_unpickle, Py_TYPE(myself), dict, bytes); @@ -3245,8 +3249,9 @@ PyCData_get(ctypes_state *st, PyObject *type, GETFUNC getfunc, PyObject *src, CDataObject *cdata = (CDataObject *)src; #endif if (getfunc) { + PyObject *res; LOCK_PTR(cdata); - PyObject *res = getfunc(adr, size); + res = getfunc(adr, size); UNLOCK_PTR(cdata); return res; } @@ -3256,8 +3261,9 @@ PyCData_get(ctypes_state *st, PyObject *type, GETFUNC getfunc, PyObject *src, return NULL; } if (info && info->getfunc && !_ctypes_simple_instance(st, type)) { + PyObject *res; LOCK_PTR(cdata); - PyObject *res = info->getfunc(adr, size); + res = info->getfunc(adr, size); UNLOCK_PTR(cdata); return res; } @@ -3276,8 +3282,9 @@ _PyCData_set(ctypes_state *st, int err; if (setfunc) { + PyObject *res; LOCK_PTR(dst); - PyObject *res = setfunc(ptr, value, size); + res = setfunc(ptr, value, size); UNLOCK_PTR(dst); return res; } @@ -3287,8 +3294,9 @@ _PyCData_set(ctypes_state *st, return NULL; } if (info && info->setfunc) { + PyObject *res; LOCK_PTR(dst); - PyObject *res = info->setfunc(ptr, value, size); + res = info->setfunc(ptr, value, size); UNLOCK_PTR(dst); return res; } @@ -3932,7 +3940,8 @@ PyCFuncPtr_FromDll(PyTypeObject *type, PyObject *args, PyObject *kwds) self->paramflags = Py_XNewRef(paramflags); - // No other threads can have this object + // No other threads can have this object, no need to + // lock it. *(void **)self->b_ptr = address; Py_INCREF(dll); Py_DECREF(ftuple); @@ -4865,9 +4874,10 @@ Array_subscript(PyObject *myself, PyObject *item) if (slicelen <= 0) return Py_GetConstant(Py_CONSTANT_EMPTY_BYTES); if (step == 1) { + PyObject *res; LOCK_PTR(self); - PyObject *res = PyBytes_FromStringAndSize(ptr + start, - slicelen); + res = PyBytes_FromStringAndSize(ptr + start, + slicelen); UNLOCK_PTR(self); return res; } @@ -4894,8 +4904,9 @@ Array_subscript(PyObject *myself, PyObject *item) if (slicelen <= 0) return Py_GetConstant(Py_CONSTANT_EMPTY_STR); if (step == 1) { + PyObject *res; LOCK_PTR(self); - PyObject *res = PyUnicode_FromWideChar(ptr + start, + res = PyUnicode_FromWideChar(ptr + start, slicelen); UNLOCK_PTR(self); return res; @@ -5201,8 +5212,9 @@ Simple_get_value(CDataObject *self, void *Py_UNUSED(ignored)) } assert(info); /* Cannot be NULL for CDataObject instances */ assert(info->getfunc); + PyObject *res; LOCK_PTR(self); - PyObject *res = info->getfunc(self->b_ptr, self->b_size); + res = info->getfunc(self->b_ptr, self->b_size); UNLOCK_PTR(self); return res; } @@ -5240,8 +5252,9 @@ static PyMethodDef Simple_methods[] = { static int Simple_bool(CDataObject *self) { + int cmp; LOCK_PTR(self); - int cmp = memcmp(self->b_ptr, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0", self->b_size); + cmp = memcmp(self->b_ptr, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0", self->b_size); UNLOCK_PTR(self); return cmp; } @@ -5571,9 +5584,10 @@ Pointer_subscript(PyObject *myself, PyObject *item) if (len <= 0) return Py_GetConstant(Py_CONSTANT_EMPTY_BYTES); if (step == 1) { + PyObject *res; LOCK_PTR(self); - PyObject *res = PyBytes_FromStringAndSize(ptr + start, - len); + res = PyBytes_FromStringAndSize(ptr + start, + len); UNLOCK_PTR(self); return res; } @@ -5596,9 +5610,10 @@ Pointer_subscript(PyObject *myself, PyObject *item) if (len <= 0) return Py_GetConstant(Py_CONSTANT_EMPTY_STR); if (step == 1) { + PyObject *res; LOCK_PTR(self); - PyObject *res = PyUnicode_FromWideChar(ptr + start, - len); + res = PyUnicode_FromWideChar(ptr + start, + len); UNLOCK_PTR(self); return res; } diff --git a/Modules/_ctypes/ctypes.h b/Modules/_ctypes/ctypes.h index 5e004403c4189a..cc09639e21f7c2 100644 --- a/Modules/_ctypes/ctypes.h +++ b/Modules/_ctypes/ctypes.h @@ -152,9 +152,6 @@ union value { struct tagCDataObject { PyObject_HEAD -#ifdef Py_GIL_DISABLED - PyMutex b_ptr_lock; -#endif char *b_ptr; /* pointer to memory block */ int b_needsfree; /* need _we_ free the memory? */ CDataObject *b_base; /* pointer to base object or NULL */ @@ -184,9 +181,6 @@ typedef struct { typedef struct { /* First part identical to tagCDataObject */ PyObject_HEAD -#ifdef Py_GIL_DISABLED - PyMutex b_ptr_lock; -#endif char *b_ptr; /* pointer to memory block */ int b_needsfree; /* need _we_ free the memory? */ CDataObject *b_base; /* pointer to base object or NULL */ @@ -550,9 +544,11 @@ PyStgInfo_Init(ctypes_state *state, PyTypeObject *type) return info; } +/* See discussion in gh-128490. The plan here is to eventually use a per-object + * lock rather than a critical section, but that work is for later. */ #ifdef Py_GIL_DISABLED -# define LOCK_PTR(self) PyMutex_Lock(&self->b_ptr_lock) -# define UNLOCK_PTR(self) PyMutex_Unlock(&self->b_ptr_lock) +# define LOCK_PTR(self) Py_BEGIN_CRITICAL_SECTION(self) +# define UNLOCK_PTR(self) Py_END_CRITICAL_SECTION() #else # define LOCK_PTR(self) # define UNLOCK_PTR(self) @@ -577,8 +573,9 @@ locked_memcpy_from(void *buf, CDataObject *self, Py_ssize_t size) static inline void * locked_deref(CDataObject *self) { + void *ptr; LOCK_PTR(self); - void *ptr = *(void **)self->b_ptr; + ptr = *(void **)self->b_ptr; UNLOCK_PTR(self); return ptr; } From ce942340bd905a6493200d673ce43151bd90f40f Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Thu, 9 Jan 2025 10:42:56 -0500 Subject: [PATCH 20/24] Update test_arrays.py Co-authored-by: Petr Viktorin --- Lib/test/test_ctypes/test_arrays.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_ctypes/test_arrays.py b/Lib/test/test_ctypes/test_arrays.py index fe76a7f382d49c..7f1f6cf58402c9 100644 --- a/Lib/test/test_ctypes/test_arrays.py +++ b/Lib/test/test_ctypes/test_arrays.py @@ -284,7 +284,8 @@ def run(): with threading_helper.start_threads(threads): pass - self.assertIsNone(cm.exc_value) + if cm.exc_value: + raise cm.exc_value if __name__ == '__main__': From d6a4bc846a5932632c40643863c4d651b79ad4ce Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Thu, 9 Jan 2025 11:08:04 -0500 Subject: [PATCH 21/24] Update Doc/library/ctypes.rst MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com> --- Doc/library/ctypes.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Doc/library/ctypes.rst b/Doc/library/ctypes.rst index 2a7977fcd9a0d3..e40fb52e312422 100644 --- a/Doc/library/ctypes.rst +++ b/Doc/library/ctypes.rst @@ -884,7 +884,7 @@ In ctypes, reads and writes to a single object concurrently is safe, but not acr >>> pointer_a = pointer(number) >>> pointer_b = pointer(number) -In the above, it's only safe for one object to read and write to the address at once if the :term:`GIL` is disabled. +In the above, it's only safe for one object to read and write to the address at once if the GIL is disabled. So, ``pointer_a`` can be shared and written to across multiple threads, but only if ``pointer_b`` is not also attempting to do the same. If this is an issue, consider using a :class:`threading.Lock` to synchronize access to memory: From 5a4c1d69263411bbcbea9a8513aba3ea1cfe9155 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Thu, 9 Jan 2025 11:08:19 -0500 Subject: [PATCH 22/24] Update Modules/_ctypes/_ctypes.c Co-authored-by: Petr Viktorin --- Modules/_ctypes/_ctypes.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index 138ef2d66f3416..ee4dcd4f8ccc30 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -3117,8 +3117,11 @@ static int PyCData_MallocBuffer(CDataObject *obj, StgInfo *info) { /* We don't have to lock in this function, because it's only - used in constructors and therefore does not have concurrent - access. */ + * used in constructors and therefore does not have concurrent + * access. + */ + assert (Py_REFCNT(obj) == 1); + if ((size_t)info->size <= sizeof(obj->b_value)) { /* No need to call malloc, can use the default buffer */ obj->b_ptr = (char *)&obj->b_value; From 774a6ec19b259888b8254f56419f36622545eb8a Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Thu, 9 Jan 2025 11:10:41 -0500 Subject: [PATCH 23/24] Update Modules/_ctypes/_ctypes.c Co-authored-by: Petr Viktorin --- Modules/_ctypes/_ctypes.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index ee4dcd4f8ccc30..495d4f9224055a 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -1553,7 +1553,7 @@ WCharArray_set_value(CDataObject *self, PyObject *value, void *Py_UNUSED(ignored PyErr_SetString(PyExc_ValueError, "string too long"); return -1; } - int rc; + Py_ssize_t rc; LOCK_PTR(self); rc = PyUnicode_AsWideChar(value, (wchar_t *)self->b_ptr, size); UNLOCK_PTR(self); From c8f2cd3e3d65987af50d4bbe418dd411b67c6921 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Thu, 9 Jan 2025 19:18:36 -0500 Subject: [PATCH 24/24] Remove silly seealso. --- Doc/library/ctypes.rst | 3 --- 1 file changed, 3 deletions(-) diff --git a/Doc/library/ctypes.rst b/Doc/library/ctypes.rst index e40fb52e312422..714051c9f57482 100644 --- a/Doc/library/ctypes.rst +++ b/Doc/library/ctypes.rst @@ -900,9 +900,6 @@ to synchronize access to memory: >>> with lock: ... pointer_b.contents = 42 -.. seealso:: - - Use :func:`sys._is_gil_enabled` to dynamically synchronize your application. .. _ctypes-type-conversions:

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier! Saves Data!