Embedded python has no clear GIL ownership contract, and the cross-thread path depends on a pybind11 2.x behavior that 3.x removes
Filing this after withdrawing #20043, where I tried to fix what looked like a local bug and found it is a contract question the sig should decide.
The observation that started it: PythonSystemComponent::PythonGILScopedLock::Lock constructs pybind11::gil_scoped_release unconditionally, including on threads that have never entered python. That constructor calls PyEval_SaveThread, which CPython requires be made with a current thread state for the calling thread. pybind11 3.x states the requirement explicitly in gil.h ("PRECONDITION: The GIL must be held when this constructor is called") and asserts on it in debug builds; the 2.10 currently pinned here carries no such note, which is part of why the pattern went unnoticed.
On the bundled pybind11 2.10 this works anyway, and the cross-thread path depends on it working. On pybind11 3.x the same call is a hard segfault, reproducible on an asset builder job thread through PythonAssetBuilder's createJobs path.
What makes it a contract question rather than a patch is what happens when you try to correct it. I measured four configurations against two existing test suites, PythonThreadingTest (which spawns a thread and dispatches an EBus event into python) and PythonReflectionPairTests (which calls pybind11 directly from the test body):
- Unmodified tree, bundled python: both pass, threading in about 24 ms.
- Guard the release with PyGILState_Check, change nothing else: reflection still passes, threading deadlocks. Thread stacks show the worker blocked in take_gil under gil_scoped_acquire while the main thread holds the GIL inside AZStd::thread::join.
- Also release the GIL after startup, which is the usual embedding pattern: threading works, reflection crashes in PyEval_GetGlobals under pybind11::globals. The test suite makes about 92 direct pybind11 calls across 13 files and none of them acquire the GIL, so they depend on the initializing thread keeping it.
- Additionally have the shared PythonTestingFixture hold the GIL for the test body and release it around the thread joins: the crash moves into shutdown, where StopPythonInterpreter blocks in PyEval_RestoreThread waiting for the GIL the fixture still holds while the entity deactivates.
Every one of those failures has the same root cause seen from a different side: there is no single place that owns the GIL between startup, the lock helper, direct pybind11 use, cross-thread dispatch, and shutdown. Engine code itself is consistent (ExecuteByString and friends take PythonGILScopedLock before touching pybind11, and PythonProxyObject's direct calls run inside python callbacks where the interpreter already holds the GIL). The ambiguity shows up at the boundaries.
Why it is worth deciding now rather than later: pybind11 just moved to FetchContent in #19566, still pinned to 2.10. Whenever that pin moves to 3.x, the current cross-thread behavior stops being merely undocumented and becomes a crash, and PythonThreadingTest is the test that will catch it.
I am not proposing a specific design here. Two directions I looked at trade off differently: releasing the GIL at rest and requiring callers to acquire is the usual embedding practice but changes what the test suite assumes and needs an owner for shutdown; keeping it held at rest and giving cross-thread dispatch an explicit handoff leaves the tests alone but adds a path to maintain. There may well be better options, and it is also a legitimate answer that this is not worth changing while the pin stays on 2.10. I can supply the reproduction for any of the four cases above, and if this is something the team decides to change I am glad to help with the work.
Source: o3de/o3de