ctypes thread safety auditing (and fixing)

[ZeroIntensity]

Feature or enhancement

This is a tracking issue for all thread safety problems related to ctypes. I'll be working on this, but others can feel free to give me some help.

First of all, we need to find where the thread safety problems are.

Auditing

Preview Give feedback

1. Audit _ctypes.c
2. Audit _ctypes_test.c (and probably the generated file too)
3. Audit callbacks.c
4. Audit callproc.c
5. Audit cfield.c
6. Audit malloc_closure.c
7. Audit stgdict.c

I'll be tracking the issues that get found when auditing here. The plan is to just create a new issue and link to it for each new problem instead of flooding this issue with PRs.

Generally, the workflow for fixes should follow most of the rules from #116738, but I suspect we'll need recursive mutexes for quite a few things related to callbacks, because it's difficult to tell what might be re-entrant, and we can't use critical sections for arbitrary function pointers.

Known Issues

Preview Give feedback

1. Crash when modifying DLL function pointers conconcurrently #127946
   3.13 3.14 extension-modules topic-ctypes topic-free-threading type-crash +6
   
2. ctypes pointer writes are not thread safe #128182
   3.13 3.14 extension-modules topic-ctypes topic-free-threading type-bug +6
3. Race in _ctypes_alloc_callback and libffi under free-threading #128485
   3.13 3.14 extension-modules topic-ctypes topic-free-threading type-bug +6
   
4. Race in ctypes PyCFuncPtr_new under free-threading #128567
   3.13 3.14 extension-modules topic-ctypes topic-free-threading type-bug +6
   
5. Race in stgdict PyCStructUnionType_update_stginfo under free-threading #128570
   3.13 3.14 extension-modules topic-ctypes topic-free-threading type-bug +6
   

cc @encukou, as the ctypes genius, and @colesbury as the free-threading mastermind.

Linked PRs

• gh-127945: add locking to malloc closure in free-threading #131662
• gh-127945: fix thread safety of ctypes state #131710
• gh-127945: fix thread safety of creating instances of ctypes structures #131716

[encukou]

Thank you! I can review the ctypes parts :)

Heads-up: I'm currently working in cfield.c/PyCField*; focus elsewhere to avoid conflicts.

[ZeroIntensity]

Got it, thanks! I'll probably just move to callbacks.c next.

[kumaraditya303]

I discussed this with Peter and moving forward I'll be taking over the thread safety work and continue it.

[kumaraditya303]

I investigated the thread safety of ctypes and here's the (incomplete) list of issues:

• Race in _ctypes_alloc_callback and libffi under free-threading #128485: race in initialization of dlmalloc in _ctypes_alloc_callback. This is fixed by allocating a small memory block and freeing it in module initialization to force initialization of the allocator and is now thread safe with gh-128485: ensure that dlmalloc initializes itself at import time in ctypes  #131633.
• the freelist in malloc_closure.c is not thread safe in free-threading. This can be fixed by adding lock around the alloc and free functions in free-threading in gh-127945: add locking to malloc closure in free-threading #131662.
• concurrent calls to PyCStructUnionType_update_stginfo needs to be made thread safe as it modifies the type's data. This can be fixed by adding critical section around reading and writing to stginfo data in free-threading as using just mutex can lead to re-entrancy deadlocks.
• the initialization and concurrent modifications of stginfo is not thread safe
• the module level functions such as resize are not thread safe and need to acquire critical section of the object being resized.

[colesbury]

I think there are also races on StgInfo.flags, in particular with the DICTFLAG_FINAL bit.

cpython/Modules/_ctypes/_ctypes.c

Lines 718 to 719 in 4596666

	info->flags &= ~DICTFLAG_FINAL; /* clear the 'final' flag in the subclass info */
	baseinfo->flags |= DICTFLAG_FINAL; /* set the 'final' flag in the baseclass info */

[kumaraditya303]

I think there are also races on StgInfo.flags, in particular with the DICTFLAG_FINAL bit.

Yes, I am covering that under "the initialization and concurrent modifications of stginfo is not thread safe".

[colesbury]

This is different and more important than concurrent modification or initialization of stginfo, and I think it should be one of the early things we should focus on. Concurrent modifications of ctypes Structures is not an important use case.

However, creating separate instances of the same structure is an important use case:

from ctypes import Structure, c_int
from concurrent.futures import ThreadPoolExecutor

class POINT(Structure):
    _fields_ = [("x", c_int), ("y", c_int)]

with ThreadPoolExecutor() as executor:
    futures = [executor.submit(POINT, i, i) for i in range(1000)]
    results = [future.result() for future in futures]

This needs to be both efficient and thread-safe.