gh-127945: fix thread safety of creating instances of ctypes structures (#131716)

diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c
index d3e717757b0645ae3ff5bb30d7c00dd66833f643..803d41630dd59ac5b61ecfffe74a6d1189280001 100644 (file)
--- 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 2b8192059a0dc2b66c1cd39dafb8aa509eb1fc6b..7ffe83dc63d6dbdaee3c8fba0cb117c430d56935 100644 (file)
--- 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 6e4f8eb21d96329a5f5bf6bc6d1621c50601ca25..c4c4ec2cc5266a3efdb8062780f04c08fe40ae1f 100644 (file)
--- 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 @@ error:
     Py_XDECREF(layout_fields);
     Py_XDECREF(layout);
     Py_XDECREF(format_spec_obj);
+    STGINFO_UNLOCK();
     return retval;
 }