mirrors/cpython
1
0
Fork 0
mirror of https://github.com/python/cpython.git synced 2025-09-27 02:39:58 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 13

gh-128182: Add per-object memory access synchronization to ctypes (GH-128490)

Browse source
This commit is contained in:
Peter Bierma 2025-01-13 08:17:38 -05:00 • committed by GitHub
parent 5044c2245c
commit 8dfc743a1d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 234 additions and 47 deletions
Download patch file Download diff file Expand all files Collapse all files

30
Doc/library/ctypes.rst
View file

@ -870,6 +870,36 @@ 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 <free threading>` builds.
In ctypes, reads and writes to a single object concurrently is safe, but not across multiple objects:
.. 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 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:
.. code-block:: pycon
>>> import threading
>>> lock = threading.Lock()
>>> # Thread 1
>>> with lock:
... pointer_a.contents = 24
>>> # Thread 2
>>> with lock:
... pointer_b.contents = 42
.. _ctypes-type-conversions:

22
Lib/test/test_ctypes/test_arrays.py
View file

@ -5,7 +5,7 @@ from ctypes import (Structure, Array, ARRAY, sizeof, addressof,
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, Py_GIL_DISABLED
from ._support import (_CData, PyCArrayType, Py_TPFLAGS_DISALLOW_INSTANTIATION,
Py_TPFLAGS_IMMUTABLETYPE)
@ -267,6 +267,26 @@ class ArrayTestCase(unittest.TestCase):
def test_large_array(self, size):
c_char * size
@threading_helper.requires_working_threading()
@unittest.skipUnless(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:
threads = (Thread(target=run) for _ in range(25))
with threading_helper.start_threads(threads):
pass
if cm.exc_value:
raise cm.exc_value
if __name__ == '__main__':
unittest.main()

2
Misc/NEWS.d/next/Library/2025-01-04-11-32-46.gh-issue-128182.SJ2Zsa.rst Normal file
View file

@ -0,0 +1,2 @@
Fix crash when using :mod:`ctypes` pointers concurrently on the :term:`free
threaded <free threading>` build.

183
Modules/_ctypes/_ctypes.c
View file

@ -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, (char *)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,26 @@ 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);
PyObject *res;
LOCK_PTR(self);
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;
PyObject *res;
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);
res = PyBytes_FromStringAndSize(self->b_ptr, i);
UNLOCK_PTR(self);
return res;
}
static int
@ -1486,9 +1493,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;
@ -1506,11 +1515,15 @@ 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;
return PyUnicode_FromWideChar((wchar_t *)self->b_ptr, i);
res = PyUnicode_FromWideChar((wchar_t *)self->b_ptr, i);
UNLOCK_PTR(self);
return res;
}
static int
@ -1540,10 +1553,11 @@ WCharArray_set_value(CDataObject *self, PyObject *value, void *Py_UNUSED(ignored
PyErr_SetString(PyExc_ValueError, "string too long");
return -1;
}
if (PyUnicode_AsWideChar(value, (wchar_t *)self->b_ptr, size) < 0) {
return -1;
}
return 0;
Py_ssize_t rc;
LOCK_PTR(self);
rc = PyUnicode_AsWideChar(value, (wchar_t *)self->b_ptr, size);
UNLOCK_PTR(self);
return rc < 0 ? -1 : 0;
}
static PyGetSetDef WCharArray_getsets[] = {
@ -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,12 @@ PyCData_reduce_impl(PyObject *myself, PyTypeObject *cls)
if (dict == NULL) {
return NULL;
}
PyObject *bytes;
LOCK_PTR(self);
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 +3055,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 +3116,12 @@ static PyType_Spec pycdata_spec = {
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.
*/
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;
@ -3219,15 +3247,28 @@ 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);
#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) {
PyObject *res;
LOCK_PTR(cdata);
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);
PyObject *res;
LOCK_PTR(cdata);
res = info->getfunc(adr, size);
UNLOCK_PTR(cdata);
return res;
}
return PyCData_FromBaseObj(st, type, src, index, adr);
}
@ -3244,15 +3285,24 @@ _PyCData_set(ctypes_state *st,
int err;
if (setfunc) {
return setfunc(ptr, value, size);
PyObject *res;
LOCK_PTR(dst);
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) {
PyObject *res;
LOCK_PTR(dst);
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!
@ -3272,7 +3322,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,
@ -3288,9 +3340,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 */
@ -3324,7 +3374,9 @@ _PyCData_set(ctypes_state *st,
((PyTypeObject *)type)->tp_name);
return NULL;
}
LOCK_PTR(src);
*(void **)ptr = src->b_ptr;
UNLOCK_PTR(src);
keep = GetKeepedObjects(src);
if (keep == NULL)
@ -3891,6 +3943,8 @@ PyCFuncPtr_FromDll(PyTypeObject *type, PyObject *args, PyObject *kwds)
self->paramflags = Py_XNewRef(paramflags);
// No other threads can have this object, no need to
// lock it.
*(void **)self->b_ptr = address;
Py_INCREF(dll);
Py_DECREF(ftuple);
@ -4823,18 +4877,24 @@ Array_subscript(PyObject *myself, PyObject *item)
if (slicelen <= 0)
return Py_GetConstant(Py_CONSTANT_EMPTY_BYTES);
if (step == 1) {
return PyBytes_FromStringAndSize(ptr + start,
slicelen);
PyObject *res;
LOCK_PTR(self);
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 +4907,12 @@ Array_subscript(PyObject *myself, PyObject *item)
if (slicelen <= 0)
return Py_GetConstant(Py_CONSTANT_EMPTY_STR);
if (step == 1) {
return PyUnicode_FromWideChar(ptr + start,
slicelen);
PyObject *res;
LOCK_PTR(self);
res = PyUnicode_FromWideChar(ptr + start,
slicelen);
UNLOCK_PTR(self);
return res;
}
dest = PyMem_New(wchar_t, slicelen);
@ -4857,10 +4921,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 +5184,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 +5215,11 @@ 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);
PyObject *res;
LOCK_PTR(self);
res = info->getfunc(self->b_ptr, self->b_size);
UNLOCK_PTR(self);
return res;
}
static PyGetSetDef Simple_getsets[] = {
@ -5183,7 +5255,11 @@ 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);
int cmp;
LOCK_PTR(self);
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 +5315,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 +5344,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, (char *)((char *)deref + offset));
}
static int
@ -5284,7 +5361,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 +5389,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, ((char *)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 +5411,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 +5446,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 +5581,53 @@ 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,
len);
PyObject *res;
LOCK_PTR(self);
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,
len);
PyObject *res;
LOCK_PTR(self);
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 +5653,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 +5861,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:

44
Modules/_ctypes/ctypes.h
View file

@ -543,3 +543,47 @@ PyStgInfo_Init(ctypes_state *state, PyTypeObject *type)
info->initialized = 1;
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) Py_BEGIN_CRITICAL_SECTION(self)
# define UNLOCK_PTR(self) Py_END_CRITICAL_SECTION()
#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);
(void)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);
(void)memcpy(buf, self->b_ptr, size);
UNLOCK_PTR(self);
}
static inline void *
locked_deref(CDataObject *self)
{
void *ptr;
LOCK_PTR(self);
ptr = *(void **)self->b_ptr;
UNLOCK_PTR(self);
return ptr;
}
static inline void
locked_deref_assign(CDataObject *self, void *new_ptr)
{
LOCK_PTR(self);
*(void **)self->b_ptr = new_ptr;
UNLOCK_PTR(self);
}