https://github.com/python/cpython/commit/bc5a028c132efd7598593b75ab86b690209a6e6b
commit: bc5a028c132efd7598593b75ab86b690209a6e6b
branch: main
author: Kumar Aditya <[email protected]>
committer: kumaraditya303 <[email protected]>
date: 2025-03-30T15:22:30+05:30
summary:
gh-127945: fix thread safety of creating instances of ctypes structures
(#131716)
files:
M Modules/_ctypes/_ctypes.c
M Modules/_ctypes/ctypes.h
M Modules/_ctypes/stgdict.c
diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c
index d3e717757b0645..803d41630dd59a 100644
--- a/Modules/_ctypes/_ctypes.c
+++ b/Modules/_ctypes/_ctypes.c
@@ -109,6 +109,7 @@ bytes(cdata)
#include "pycore_call.h" // _PyObject_CallNoArgs()
#include "pycore_ceval.h" // _Py_EnterRecursiveCall()
#include "pycore_unicodeobject.h" // _PyUnicode_EqualToASCIIString()
+#include "pycore_pyatomic_ft_wrappers.h"
#ifdef MS_WIN32
# include "pycore_modsupport.h" // _PyArg_NoKeywords()
#endif
@@ -710,13 +711,15 @@ StructUnionType_init(PyObject *self, PyObject *args,
PyObject *kwds, int isStruc
if (baseinfo == NULL) {
return 0;
}
-
+ int ret = 0;
+ STGINFO_LOCK(baseinfo);
/* copy base info */
- if (PyCStgInfo_clone(info, baseinfo) < 0) {
- return -1;
+ ret = PyCStgInfo_clone(info, baseinfo);
+ if (ret >= 0) {
+ stginfo_set_dict_final_lock_held(baseinfo); /* set the 'final'
flag in the baseclass info */
}
- info->flags &= ~DICTFLAG_FINAL; /* clear the 'final' flag in the
subclass info */
- baseinfo->flags |= DICTFLAG_FINAL; /* set the 'final' flag in the
baseclass info */
+ STGINFO_UNLOCK();
+ return ret;
}
return 0;
}
@@ -3122,6 +3125,7 @@ PyCData_MallocBuffer(CDataObject *obj, StgInfo *info)
* access.
*/
assert (Py_REFCNT(obj) == 1);
+ assert(stginfo_get_dict_final(info) == 1);
if ((size_t)info->size <= sizeof(obj->b_value)) {
/* No need to call malloc, can use the default buffer */
@@ -3167,7 +3171,7 @@ PyCData_FromBaseObj(ctypes_state *st,
return NULL;
}
- info->flags |= DICTFLAG_FINAL;
+ stginfo_set_dict_final(info);
cmem = (CDataObject *)((PyTypeObject *)type)->tp_alloc((PyTypeObject
*)type, 0);
if (cmem == NULL) {
return NULL;
@@ -3216,7 +3220,7 @@ PyCData_AtAddress(ctypes_state *st, PyObject *type, void
*buf)
return NULL;
}
- info->flags |= DICTFLAG_FINAL;
+ stginfo_set_dict_final(info);
pd = (CDataObject *)((PyTypeObject *)type)->tp_alloc((PyTypeObject *)type,
0);
if (!pd) {
@@ -3451,7 +3455,7 @@ generic_pycdata_new(ctypes_state *st,
return NULL;
}
- info->flags |= DICTFLAG_FINAL;
+ stginfo_set_dict_final(info);
obj = (CDataObject *)type->tp_alloc(type, 0);
if (!obj)
diff --git a/Modules/_ctypes/ctypes.h b/Modules/_ctypes/ctypes.h
index 2b8192059a0dc2..7ffe83dc63d6db 100644
--- a/Modules/_ctypes/ctypes.h
+++ b/Modules/_ctypes/ctypes.h
@@ -6,6 +6,8 @@
#include "pycore_moduleobject.h" // _PyModule_GetState()
#include "pycore_typeobject.h" // _PyType_GetModuleState()
+#include "pycore_critical_section.h"
+#include "pycore_pyatomic_ft_wrappers.h"
// Do we support C99 complex types in ffi?
// For Apple's libffi, this must be determined at runtime (see gh-128156).
@@ -375,7 +377,7 @@ typedef struct CFieldObject {
typedef struct {
int initialized;
Py_ssize_t size; /* number of bytes */
- Py_ssize_t align; /* alignment requirements */
+ Py_ssize_t align; /* alignment reqwuirements */
Py_ssize_t length; /* number of fields */
ffi_type ffi_type_pointer;
PyObject *proto; /* Only for Pointer/ArrayObject */
@@ -390,15 +392,64 @@ typedef struct {
PyObject *checker;
PyObject *module;
int flags; /* calling convention and such */
+#ifdef Py_GIL_DISABLED
+ PyMutex mutex; /* critical section mutex */
+#endif
+ uint8_t dict_final;
/* pep3118 fields, pointers need PyMem_Free */
char *format;
int ndim;
Py_ssize_t *shape;
-/* Py_ssize_t *strides; */ /* unused in ctypes */
-/* Py_ssize_t *suboffsets; */ /* unused in ctypes */
+ /* Py_ssize_t *strides; */ /* unused in ctypes */
+ /* Py_ssize_t *suboffsets; */ /* unused in ctypes */
} StgInfo;
+
+/*
+ To ensure thread safety in the free threading build, the `STGINFO_LOCK` and
+ `STGINFO_UNLOCK` macros use critical sections to protect against concurrent
+ modifications to `StgInfo` and assignment of the `dict_final` field. Once
+ `dict_final` is set, `StgInfo` is treated as read-only, and no further
+ modifications are allowed. This approach allows most read operations to
+ proceed without acquiring the critical section lock.
+
+ The `dict_final` field is written only after all other modifications to
+ `StgInfo` are complete. The reads and writes of `dict_final` use the
+ sequentially consistent memory ordering to ensure that all other fields are
+ visible to other threads before the `dict_final` bit is set.
+*/
+
+#define STGINFO_LOCK(stginfo)
Py_BEGIN_CRITICAL_SECTION_MUT(&(stginfo)->mutex)
+#define STGINFO_UNLOCK() Py_END_CRITICAL_SECTION()
+
+static inline uint8_t
+stginfo_get_dict_final(StgInfo *info)
+{
+ return FT_ATOMIC_LOAD_UINT8(info->dict_final);
+}
+
+static inline void
+stginfo_set_dict_final_lock_held(StgInfo *info)
+{
+ _Py_CRITICAL_SECTION_ASSERT_MUTEX_LOCKED(&info->mutex);
+ FT_ATOMIC_STORE_UINT8(info->dict_final, 1);
+}
+
+
+// Set the `dict_final` bit in StgInfo. It checks if the bit is already set
+// and in that avoids acquiring the critical section (general case).
+static inline void
+stginfo_set_dict_final(StgInfo *info)
+{
+ if (stginfo_get_dict_final(info) == 1) {
+ return;
+ }
+ STGINFO_LOCK(info);
+ stginfo_set_dict_final_lock_held(info);
+ STGINFO_UNLOCK();
+}
+
extern int PyCStgInfo_clone(StgInfo *dst_info, StgInfo *src_info);
extern void ctype_clear_stginfo(StgInfo *info);
@@ -427,8 +478,6 @@ PyObject *_ctypes_callproc(ctypes_state *st,
#define TYPEFLAG_ISPOINTER 0x100
#define TYPEFLAG_HASPOINTER 0x200
-#define DICTFLAG_FINAL 0x1000
-
struct tagPyCArgObject {
PyObject_HEAD
ffi_type *pffi_type;
diff --git a/Modules/_ctypes/stgdict.c b/Modules/_ctypes/stgdict.c
index 6e4f8eb21d9632..c4c4ec2cc5266a 100644
--- a/Modules/_ctypes/stgdict.c
+++ b/Modules/_ctypes/stgdict.c
@@ -34,6 +34,10 @@ PyCStgInfo_clone(StgInfo *dst_info, StgInfo *src_info)
dst_info->ffi_type_pointer.elements = NULL;
memcpy(dst_info, src_info, sizeof(StgInfo));
+#ifdef Py_GIL_DISABLED
+ dst_info->mutex = (PyMutex){0};
+#endif
+ dst_info->dict_final = 0;
Py_XINCREF(dst_info->proto);
Py_XINCREF(dst_info->argtypes);
@@ -248,23 +252,23 @@ PyCStructUnionType_update_stginfo(PyObject *type,
PyObject *fields, int isStruct
ctypes_state *st = get_module_state_by_def(Py_TYPE(type));
StgInfo *stginfo;
if (PyStgInfo_FromType(st, type, &stginfo) < 0) {
- goto error;
+ return -1;
}
if (!stginfo) {
PyErr_SetString(PyExc_TypeError,
"ctypes state is not initialized");
- goto error;
+ return -1;
}
PyObject *base = (PyObject *)((PyTypeObject *)type)->tp_base;
StgInfo *baseinfo;
if (PyStgInfo_FromType(st, base, &baseinfo) < 0) {
- goto error;
+ return -1;
}
+ STGINFO_LOCK(stginfo);
/* If this structure/union is already marked final we cannot assign
_fields_ anymore. */
-
- if (stginfo->flags & DICTFLAG_FINAL) {/* is final ? */
+ if (stginfo_get_dict_final(stginfo) == 1) {/* is final ? */
PyErr_SetString(PyExc_AttributeError,
"_fields_ is final");
goto error;
@@ -422,12 +426,13 @@ PyCStructUnionType_update_stginfo(PyObject *type,
PyObject *fields, int isStruct
goto error;
}
assert(info);
-
+ STGINFO_LOCK(info);
stginfo->ffi_type_pointer.elements[ffi_ofs + i] =
&info->ffi_type_pointer;
if (info->flags & (TYPEFLAG_ISPOINTER | TYPEFLAG_HASPOINTER))
stginfo->flags |= TYPEFLAG_HASPOINTER;
- info->flags |= DICTFLAG_FINAL; /* mark field type final */
+ stginfo_set_dict_final_lock_held(info); /* mark field type final */
+ STGINFO_UNLOCK();
if (-1 == PyObject_SetAttr(type, prop->name, prop_obj)) {
goto error;
}
@@ -461,15 +466,15 @@ PyCStructUnionType_update_stginfo(PyObject *type,
PyObject *fields, int isStruct
/* We did check that this flag was NOT set above, it must not
have been set until now. */
- if (stginfo->flags & DICTFLAG_FINAL) {
+ if (stginfo_get_dict_final(stginfo) == 1) {
PyErr_SetString(PyExc_AttributeError,
"Structure or union cannot contain itself");
goto error;
}
- stginfo->flags |= DICTFLAG_FINAL;
+ stginfo_set_dict_final_lock_held(stginfo);
retval = MakeAnonFields(type);
-error:
+error:;
Py_XDECREF(layout_func);
Py_XDECREF(kwnames);
Py_XDECREF(align_obj);
@@ -478,6 +483,7 @@ PyCStructUnionType_update_stginfo(PyObject *type, PyObject
*fields, int isStruct
Py_XDECREF(layout_fields);
Py_XDECREF(layout);
Py_XDECREF(format_spec_obj);
+ STGINFO_UNLOCK();
return retval;
}
_______________________________________________
Python-checkins mailing list -- [email protected]
To unsubscribe send an email to [email protected]
https://mail.python.org/mailman3/lists/python-checkins.python.org/
Member address: [email protected]
