Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit bc5a028

Browse filesBrowse files
kumaraditya303kumaraditya303
authored
gh-127945: fix thread safety of creating instances of ctypes structures (#131716)
1 parent edfbd8c commit bc5a028
Copy full SHA for bc5a028

File tree

Expand file treeCollapse file tree

3 files changed

+82
-23
lines changed
Filter options
Expand file treeCollapse file tree

3 files changed

+82
-23
lines changed

‎Modules/_ctypes/_ctypes.c

Copy file name to clipboardExpand all lines: Modules/_ctypes/_ctypes.c
+12-8Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ bytes(cdata)
109109
#include "pycore_call.h" // _PyObject_CallNoArgs()
110110
#include "pycore_ceval.h" // _Py_EnterRecursiveCall()
111111
#include "pycore_unicodeobject.h" // _PyUnicode_EqualToASCIIString()
112+
#include "pycore_pyatomic_ft_wrappers.h"
112113
#ifdef MS_WIN32
113114
# include "pycore_modsupport.h" // _PyArg_NoKeywords()
114115
#endif
@@ -710,13 +711,15 @@ StructUnionType_init(PyObject *self, PyObject *args, PyObject *kwds, int isStruc
710711
if (baseinfo == NULL) {
711712
return 0;
712713
}
713-
714+
int ret = 0;
715+
STGINFO_LOCK(baseinfo);
714716
/* copy base info */
715-
if (PyCStgInfo_clone(info, baseinfo) < 0) {
716-
return -1;
717+
ret = PyCStgInfo_clone(info, baseinfo);
718+
if (ret >= 0) {
719+
stginfo_set_dict_final_lock_held(baseinfo); /* set the 'final' flag in the baseclass info */
717720
}
718-
info->flags &= ~DICTFLAG_FINAL; /* clear the 'final' flag in the subclass info */
719-
baseinfo->flags |= DICTFLAG_FINAL; /* set the 'final' flag in the baseclass info */
721+
STGINFO_UNLOCK();
722+
return ret;
720723
}
721724
return 0;
722725
}
@@ -3122,6 +3125,7 @@ PyCData_MallocBuffer(CDataObject *obj, StgInfo *info)
31223125
* access.
31233126
*/
31243127
assert (Py_REFCNT(obj) == 1);
3128+
assert(stginfo_get_dict_final(info) == 1);
31253129

31263130
if ((size_t)info->size <= sizeof(obj->b_value)) {
31273131
/* No need to call malloc, can use the default buffer */
@@ -3167,7 +3171,7 @@ PyCData_FromBaseObj(ctypes_state *st,
31673171
return NULL;
31683172
}
31693173

3170-
info->flags |= DICTFLAG_FINAL;
3174+
stginfo_set_dict_final(info);
31713175
cmem = (CDataObject *)((PyTypeObject *)type)->tp_alloc((PyTypeObject *)type, 0);
31723176
if (cmem == NULL) {
31733177
return NULL;
@@ -3216,7 +3220,7 @@ PyCData_AtAddress(ctypes_state *st, PyObject *type, void *buf)
32163220
return NULL;
32173221
}
32183222

3219-
info->flags |= DICTFLAG_FINAL;
3223+
stginfo_set_dict_final(info);
32203224

32213225
pd = (CDataObject *)((PyTypeObject *)type)->tp_alloc((PyTypeObject *)type, 0);
32223226
if (!pd) {
@@ -3451,7 +3455,7 @@ generic_pycdata_new(ctypes_state *st,
34513455
return NULL;
34523456
}
34533457

3454-
info->flags |= DICTFLAG_FINAL;
3458+
stginfo_set_dict_final(info);
34553459

34563460
obj = (CDataObject *)type->tp_alloc(type, 0);
34573461
if (!obj)

‎Modules/_ctypes/ctypes.h

Copy file name to clipboardExpand all lines: Modules/_ctypes/ctypes.h
+54-5Lines changed: 54 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66

77
#include "pycore_moduleobject.h" // _PyModule_GetState()
88
#include "pycore_typeobject.h" // _PyType_GetModuleState()
9+
#include "pycore_critical_section.h"
10+
#include "pycore_pyatomic_ft_wrappers.h"
911

1012
// Do we support C99 complex types in ffi?
1113
// For Apple's libffi, this must be determined at runtime (see gh-128156).
@@ -375,7 +377,7 @@ typedef struct CFieldObject {
375377
typedef struct {
376378
int initialized;
377379
Py_ssize_t size; /* number of bytes */
378-
Py_ssize_t align; /* alignment requirements */
380+
Py_ssize_t align; /* alignment reqwuirements */
379381
Py_ssize_t length; /* number of fields */
380382
ffi_type ffi_type_pointer;
381383
PyObject *proto; /* Only for Pointer/ArrayObject */
@@ -390,15 +392,64 @@ typedef struct {
390392
PyObject *checker;
391393
PyObject *module;
392394
int flags; /* calling convention and such */
395+
#ifdef Py_GIL_DISABLED
396+
PyMutex mutex; /* critical section mutex */
397+
#endif
398+
uint8_t dict_final;
393399

394400
/* pep3118 fields, pointers need PyMem_Free */
395401
char *format;
396402
int ndim;
397403
Py_ssize_t *shape;
398-
/* Py_ssize_t *strides; */ /* unused in ctypes */
399-
/* Py_ssize_t *suboffsets; */ /* unused in ctypes */
404+
/* Py_ssize_t *strides; */ /* unused in ctypes */
405+
/* Py_ssize_t *suboffsets; */ /* unused in ctypes */
400406
} StgInfo;
401407

408+
409+
/*
410+
To ensure thread safety in the free threading build, the `STGINFO_LOCK` and
411+
`STGINFO_UNLOCK` macros use critical sections to protect against concurrent
412+
modifications to `StgInfo` and assignment of the `dict_final` field. Once
413+
`dict_final` is set, `StgInfo` is treated as read-only, and no further
414+
modifications are allowed. This approach allows most read operations to
415+
proceed without acquiring the critical section lock.
416+
417+
The `dict_final` field is written only after all other modifications to
418+
`StgInfo` are complete. The reads and writes of `dict_final` use the
419+
sequentially consistent memory ordering to ensure that all other fields are
420+
visible to other threads before the `dict_final` bit is set.
421+
*/
422+
423+
#define STGINFO_LOCK(stginfo) Py_BEGIN_CRITICAL_SECTION_MUT(&(stginfo)->mutex)
424+
#define STGINFO_UNLOCK() Py_END_CRITICAL_SECTION()
425+
426+
static inline uint8_t
427+
stginfo_get_dict_final(StgInfo *info)
428+
{
429+
return FT_ATOMIC_LOAD_UINT8(info->dict_final);
430+
}
431+
432+
static inline void
433+
stginfo_set_dict_final_lock_held(StgInfo *info)
434+
{
435+
_Py_CRITICAL_SECTION_ASSERT_MUTEX_LOCKED(&info->mutex);
436+
FT_ATOMIC_STORE_UINT8(info->dict_final, 1);
437+
}
438+
439+
440+
// Set the `dict_final` bit in StgInfo. It checks if the bit is already set
441+
// and in that avoids acquiring the critical section (general case).
442+
static inline void
443+
stginfo_set_dict_final(StgInfo *info)
444+
{
445+
if (stginfo_get_dict_final(info) == 1) {
446+
return;
447+
}
448+
STGINFO_LOCK(info);
449+
stginfo_set_dict_final_lock_held(info);
450+
STGINFO_UNLOCK();
451+
}
452+
402453
extern int PyCStgInfo_clone(StgInfo *dst_info, StgInfo *src_info);
403454
extern void ctype_clear_stginfo(StgInfo *info);
404455

@@ -427,8 +478,6 @@ PyObject *_ctypes_callproc(ctypes_state *st,
427478
#define TYPEFLAG_ISPOINTER 0x100
428479
#define TYPEFLAG_HASPOINTER 0x200
429480

430-
#define DICTFLAG_FINAL 0x1000
431-
432481
struct tagPyCArgObject {
433482
PyObject_HEAD
434483
ffi_type *pffi_type;

‎Modules/_ctypes/stgdict.c

Copy file name to clipboardExpand all lines: Modules/_ctypes/stgdict.c
+16-10Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,10 @@ PyCStgInfo_clone(StgInfo *dst_info, StgInfo *src_info)
3434
dst_info->ffi_type_pointer.elements = NULL;
3535

3636
memcpy(dst_info, src_info, sizeof(StgInfo));
37+
#ifdef Py_GIL_DISABLED
38+
dst_info->mutex = (PyMutex){0};
39+
#endif
40+
dst_info->dict_final = 0;
3741

3842
Py_XINCREF(dst_info->proto);
3943
Py_XINCREF(dst_info->argtypes);
@@ -248,23 +252,23 @@ PyCStructUnionType_update_stginfo(PyObject *type, PyObject *fields, int isStruct
248252
ctypes_state *st = get_module_state_by_def(Py_TYPE(type));
249253
StgInfo *stginfo;
250254
if (PyStgInfo_FromType(st, type, &stginfo) < 0) {
251-
goto error;
255+
return -1;
252256
}
253257
if (!stginfo) {
254258
PyErr_SetString(PyExc_TypeError,
255259
"ctypes state is not initialized");
256-
goto error;
260+
return -1;
257261
}
258262
PyObject *base = (PyObject *)((PyTypeObject *)type)->tp_base;
259263
StgInfo *baseinfo;
260264
if (PyStgInfo_FromType(st, base, &baseinfo) < 0) {
261-
goto error;
265+
return -1;
262266
}
263267

268+
STGINFO_LOCK(stginfo);
264269
/* If this structure/union is already marked final we cannot assign
265270
_fields_ anymore. */
266-
267-
if (stginfo->flags & DICTFLAG_FINAL) {/* is final ? */
271+
if (stginfo_get_dict_final(stginfo) == 1) {/* is final ? */
268272
PyErr_SetString(PyExc_AttributeError,
269273
"_fields_ is final");
270274
goto error;
@@ -422,12 +426,13 @@ PyCStructUnionType_update_stginfo(PyObject *type, PyObject *fields, int isStruct
422426
goto error;
423427
}
424428
assert(info);
425-
429+
STGINFO_LOCK(info);
426430
stginfo->ffi_type_pointer.elements[ffi_ofs + i] = &info->ffi_type_pointer;
427431
if (info->flags & (TYPEFLAG_ISPOINTER | TYPEFLAG_HASPOINTER))
428432
stginfo->flags |= TYPEFLAG_HASPOINTER;
429-
info->flags |= DICTFLAG_FINAL; /* mark field type final */
430433

434+
stginfo_set_dict_final_lock_held(info); /* mark field type final */
435+
STGINFO_UNLOCK();
431436
if (-1 == PyObject_SetAttr(type, prop->name, prop_obj)) {
432437
goto error;
433438
}
@@ -461,15 +466,15 @@ PyCStructUnionType_update_stginfo(PyObject *type, PyObject *fields, int isStruct
461466

462467
/* We did check that this flag was NOT set above, it must not
463468
have been set until now. */
464-
if (stginfo->flags & DICTFLAG_FINAL) {
469+
if (stginfo_get_dict_final(stginfo) == 1) {
465470
PyErr_SetString(PyExc_AttributeError,
466471
"Structure or union cannot contain itself");
467472
goto error;
468473
}
469-
stginfo->flags |= DICTFLAG_FINAL;
474+
stginfo_set_dict_final_lock_held(stginfo);
470475

471476
retval = MakeAnonFields(type);
472-
error:
477+
error:;
473478
Py_XDECREF(layout_func);
474479
Py_XDECREF(kwnames);
475480
Py_XDECREF(align_obj);
@@ -478,6 +483,7 @@ PyCStructUnionType_update_stginfo(PyObject *type, PyObject *fields, int isStruct
478483
Py_XDECREF(layout_fields);
479484
Py_XDECREF(layout);
480485
Py_XDECREF(format_spec_obj);
486+
STGINFO_UNLOCK();
481487
return retval;
482488
}
483489

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.