Skip to content

gh-127946: Use a critical section for CFuncPtr attributes #128109

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Dec 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion Lib/test/test_ctypes/test_cfuncs.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
c_short, c_ushort, c_int, c_uint,
c_long, c_ulong, c_longlong, c_ulonglong,
c_float, c_double, c_longdouble)
from test.support import import_helper
from test import support
from test.support import import_helper, threading_helper
_ctypes_test = import_helper.import_module("_ctypes_test")


Expand Down Expand Up @@ -191,6 +192,23 @@ def test_void(self):
self.assertEqual(self._dll.tv_i(-42), None)
self.assertEqual(self.S(), -42)

@threading_helper.requires_working_threading()
@support.requires_resource("cpu")
@unittest.skipUnless(support.Py_GIL_DISABLED, "only meaningful on free-threading")
def test_thread_safety(self):
from threading import Thread

def concurrent():
for _ in range(100):
self._dll.tf_b.restype = c_byte
self._dll.tf_b.argtypes = (c_byte,)

with threading_helper.catch_threading_exception() as exc:
with threading_helper.start_threads((Thread(target=concurrent) for _ in range(10))):
pass

self.assertIsNone(exc.exc_value)


# The following repeats the above tests with stdcall functions (where
# they are available)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix crash when modifying :class:`ctypes._CFuncPtr` objects concurrently on
the :term:`free threaded <free threading>` build.
100 changes: 72 additions & 28 deletions Modules/_ctypes/_ctypes.c
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,9 @@ bytes(cdata)

/*[clinic input]
module _ctypes
class _ctypes.CFuncPtr "PyCFuncPtrObject *" "&PyCFuncPtr_Type"
[clinic start generated code]*/
/*[clinic end generated code: output=da39a3ee5e6b4b0d input=476a19c49b31a75c]*/
/*[clinic end generated code: output=da39a3ee5e6b4b0d input=58e8c99474bc631e]*/

#define clinic_state() (get_module_state_by_class(cls))
#define clinic_state_sub() (get_module_state_by_class(cls->tp_base))
Expand Down Expand Up @@ -3422,33 +3423,56 @@ generic_pycdata_new(ctypes_state *st,
PyCFuncPtr_Type
*/

/*[clinic input]
@critical_section
@setter
_ctypes.CFuncPtr.errcheck
[clinic start generated code]*/

static int
PyCFuncPtr_set_errcheck(PyCFuncPtrObject *self, PyObject *ob, void *Py_UNUSED(ignored))
_ctypes_CFuncPtr_errcheck_set_impl(PyCFuncPtrObject *self, PyObject *value)
/*[clinic end generated code: output=6580cf1ffdf3b9fb input=84930bb16c490b33]*/
{
if (ob && !PyCallable_Check(ob)) {
if (value && !PyCallable_Check(value)) {
PyErr_SetString(PyExc_TypeError,
"the errcheck attribute must be callable");
return -1;
}
Py_XINCREF(ob);
Py_XSETREF(self->errcheck, ob);
Py_XINCREF(value);
Py_XSETREF(self->errcheck, value);
return 0;
}

/*[clinic input]
@critical_section
@getter
_ctypes.CFuncPtr.errcheck

a function to check for errors
[clinic start generated code]*/

static PyObject *
PyCFuncPtr_get_errcheck(PyCFuncPtrObject *self, void *Py_UNUSED(ignored))
_ctypes_CFuncPtr_errcheck_get_impl(PyCFuncPtrObject *self)
/*[clinic end generated code: output=dfa6fb5c6f90fd14 input=4672135fef37819f]*/
{
if (self->errcheck) {
return Py_NewRef(self->errcheck);
}
Py_RETURN_NONE;
}

/*[clinic input]
@setter
@critical_section
_ctypes.CFuncPtr.restype
[clinic start generated code]*/

static int
PyCFuncPtr_set_restype(PyCFuncPtrObject *self, PyObject *ob, void *Py_UNUSED(ignored))
_ctypes_CFuncPtr_restype_set_impl(PyCFuncPtrObject *self, PyObject *value)
/*[clinic end generated code: output=0be0a086abbabf18 input=683c3bef4562ccc6]*/
{
PyObject *checker, *oldchecker;
if (ob == NULL) {
if (value == NULL) {
oldchecker = self->checker;
self->checker = NULL;
Py_CLEAR(self->restype);
Expand All @@ -3457,27 +3481,36 @@ PyCFuncPtr_set_restype(PyCFuncPtrObject *self, PyObject *ob, void *Py_UNUSED(ign
}
ctypes_state *st = get_module_state_by_def(Py_TYPE(Py_TYPE(self)));
StgInfo *info;
if (PyStgInfo_FromType(st, ob, &info) < 0) {
if (PyStgInfo_FromType(st, value, &info) < 0) {
return -1;
}
if (ob != Py_None && !info && !PyCallable_Check(ob)) {
if (value != Py_None && !info && !PyCallable_Check(value)) {
PyErr_SetString(PyExc_TypeError,
"restype must be a type, a callable, or None");
return -1;
}
if (PyObject_GetOptionalAttr(ob, &_Py_ID(_check_retval_), &checker) < 0) {
if (PyObject_GetOptionalAttr(value, &_Py_ID(_check_retval_), &checker) < 0) {
return -1;
}
oldchecker = self->checker;
self->checker = checker;
Py_INCREF(ob);
Py_XSETREF(self->restype, ob);
Py_INCREF(value);
Py_XSETREF(self->restype, value);
Py_XDECREF(oldchecker);
return 0;
}

/*[clinic input]
@getter
@critical_section
_ctypes.CFuncPtr.restype

specify the result type
[clinic start generated code]*/

static PyObject *
PyCFuncPtr_get_restype(PyCFuncPtrObject *self, void *Py_UNUSED(ignored))
_ctypes_CFuncPtr_restype_get_impl(PyCFuncPtrObject *self)
/*[clinic end generated code: output=c8f44cd16f1dee5e input=5e3ed95116204fd2]*/
{
if (self->restype) {
return Py_NewRef(self->restype);
Expand All @@ -3495,28 +3528,44 @@ PyCFuncPtr_get_restype(PyCFuncPtrObject *self, void *Py_UNUSED(ignored))
}
}

/*[clinic input]
@setter
@critical_section
_ctypes.CFuncPtr.argtypes
[clinic start generated code]*/

static int
PyCFuncPtr_set_argtypes(PyCFuncPtrObject *self, PyObject *ob, void *Py_UNUSED(ignored))
_ctypes_CFuncPtr_argtypes_set_impl(PyCFuncPtrObject *self, PyObject *value)
/*[clinic end generated code: output=596a36e2ae89d7d1 input=c4627573e980aa8b]*/
{
PyObject *converters;

if (ob == NULL || ob == Py_None) {
if (value == NULL || value == Py_None) {
Py_CLEAR(self->converters);
Py_CLEAR(self->argtypes);
} else {
ctypes_state *st = get_module_state_by_def(Py_TYPE(Py_TYPE(self)));
converters = converters_from_argtypes(st, ob);
converters = converters_from_argtypes(st, value);
if (!converters)
return -1;
Py_XSETREF(self->converters, converters);
Py_INCREF(ob);
Py_XSETREF(self->argtypes, ob);
Py_INCREF(value);
Py_XSETREF(self->argtypes, value);
}
return 0;
}

/*[clinic input]
@getter
@critical_section
_ctypes.CFuncPtr.argtypes

specify the argument types
[clinic start generated code]*/

static PyObject *
PyCFuncPtr_get_argtypes(PyCFuncPtrObject *self, void *Py_UNUSED(ignored))
_ctypes_CFuncPtr_argtypes_get_impl(PyCFuncPtrObject *self)
/*[clinic end generated code: output=c46b05a1b0f99172 input=37a8a545a56f8ae2]*/
{
if (self->argtypes) {
return Py_NewRef(self->argtypes);
Expand All @@ -3535,13 +3584,9 @@ PyCFuncPtr_get_argtypes(PyCFuncPtrObject *self, void *Py_UNUSED(ignored))
}

static PyGetSetDef PyCFuncPtr_getsets[] = {
{ "errcheck", (getter)PyCFuncPtr_get_errcheck, (setter)PyCFuncPtr_set_errcheck,
"a function to check for errors", NULL },
{ "restype", (getter)PyCFuncPtr_get_restype, (setter)PyCFuncPtr_set_restype,
"specify the result type", NULL },
{ "argtypes", (getter)PyCFuncPtr_get_argtypes,
(setter)PyCFuncPtr_set_argtypes,
"specify the argument types", NULL },
_CTYPES_CFUNCPTR_ERRCHECK_GETSETDEF
_CTYPES_CFUNCPTR_RESTYPE_GETSETDEF
_CTYPES_CFUNCPTR_ARGTYPES_GETSETDEF
{ NULL, NULL }
};

Expand Down Expand Up @@ -5054,7 +5099,6 @@ class _ctypes.Simple "PyObject *" "clinic_state()->Simple_Type"
[clinic start generated code]*/
/*[clinic end generated code: output=da39a3ee5e6b4b0d input=016c476c7aa8b8a8]*/


static int
Simple_set_value(CDataObject *self, PyObject *value, void *Py_UNUSED(ignored))
{
Expand Down
Loading
Loading