Test and ship free-threaded Python support deliberately - #58
Merged
Merged
Conversation
PyO3 0.28 made gil_used = false the default, so this module has declared that it runs without the GIL since the 0.7.0 upgrade, and 0.10.0 already published cp314t and cp315t wheels for Linux because the manylinux images carry the free-threaded interpreters. Nothing had ever run the tests there. - CI gains a python3.14t leg that runs the whole suite with PYTHON_GIL=0, and pins uv to the matrix interpreter (UV_PYTHON) so it cannot fall back to a GIL build of the same version. The macOS and Windows x64 wheel jobs install 3.14t and build the free-threaded wheel explicitly, since those runners have no such interpreter for --find-interpreter to discover. - The gil_used = false declaration is now explicit, and compile-time assertions pin cel-rust's Program, Context, Value and Env as Send + Sync so an upstream change cannot silently reintroduce a data race into a free-threaded wheel. - Program and OptionalValue are frozen: they have no mutating methods, and sharing them between threads now involves no borrow tracking. - pyo3_log::init() panicked if the module was initialised twice in one process; try_init() tolerates the second logger installation. - The Context environment cache is locked with lock_py_attached, so a thread waiting for it cannot stall a free-threaded interpreter's stop-the-world pause while the holder runs Python API under the lock. - tests/test_free_threading.py checks, in a fresh interpreter without PYTHON_GIL, that importing cel does not make CPython re-enable the GIL, and pins the concurrency contract for Context: concurrent evaluation is safe and consistent; concurrent mutation raises "Already borrowed" rather than racing. - Documented the threading contract; maturin floor raised to 1.14, the first release that discovers free-threaded interpreters. Claude-Session: https://claude.ai/code/session_019WbvXZFm8Nb2LXF2kiWoWW
Found by the new free-threaded CI leg. prepare_environment extracted a PyRef<Context> and, when the extract failed, fell through to the dict check and finally to "evaluation_context must be a Context object or a dict". On a free-threaded interpreter the extract fails whenever another thread is inside a mutator holding the exclusive borrow, so a reader racing an add_variable got a ValueError claiming its Context was not a Context. Check the type first, then take the borrow with a short yield-retry (a mutator holds it only for one call), and if it still cannot be taken raise a RuntimeError that names the concurrent modification. Verified on python3.14t with PYTHON_GIL=0: 566 passed across three runs. Claude-Session: https://claude.ai/code/session_019WbvXZFm8Nb2LXF2kiWoWW
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
PR 1 of the plan in #45. Free-threaded support is already shipping by accident: PyO3 0.28 made
gil_used = falsethe default, so this module has declared it runs without the GIL since 0.7.0, and 0.10.0 already publishescp314t/cp315twheels for every Linux architecture because the manylinux images carry those interpreters. Nothing had ever run the tests on a free-threaded build. This PR makes the support deliberate, tested, and complete across platforms.What
CI
3.14tentry in the test matrix, running the whole suite withPYTHON_GIL=0so a dependency lacking the declaration cannot quietly re-enable the GIL for the run.UV_PYTHONpinned to the matrix interpreter at job level. Without ituv runre-resolves to whichever compatible Python it prefers; while validating this locally, an unpinneduv runsilently replaced the 3.14t venv with 3.11.3.14tviasetup-pythonand build the free-threaded wheel explicitly (-i <python-path>), after the--find-interpreterbuild so it is not built twice. Linux needs nothing: the manylinux images already provide it.Rust
#[pymodule(gil_used = false)]spelled out, with the reasoning next to it.Send + Syncassertions for cel-rust'sProgram,Context<'static>,ValueandEnv, so an upstream bump that loses a bound fails the build instead of shipping a data race in a free-threaded wheel.ProgramandOptionalValuearefrozen: no mutating methods existed, and sharing them between threads now involves no borrow tracking.pyo3_log::init()→try_init():initpanics if the module is initialised twice in one process.Contextcache mutex is taken withMutexExt::lock_py_attached, sinceclone_refruns under it.prepare_environmentextracted aPyRef<Context>and fell through to "evaluation_context must be a Context object or a dict" when the extract failed. On 3.14t that happens whenever another thread is inside a mutator, so a reader racing anadd_variablegot aValueErrorclaiming itsContextwas not aContext. Now: type check first, short yield-retry for the borrow (a mutator holds it for one call), then aRuntimeErrornaming the concurrent modification.Tests (
tests/test_free_threading.py)PYTHON_GIL, importingcelleavessys._is_gil_enabled()false and emits noRuntimeWarning(warnings as errors). This is the test that would catch losing the declaration._is_gil_enabled()is false underPYTHON_GIL=0).add_variableracing 4 readers × 500executeon oneContext: every observed value is one a writer stored; the only permitted failure is a borrowRuntimeError.Program/OptionalValueimmutability; 8 threads sharing one frozenProgramand oneContextwith a Python callback agree on every result.Docs: the threading contract for
ContextandProgramin the API reference and the class docstring; CHANGELOG. Maturin floor raised to 1.14 (first release to discover free-threaded interpreters).Verification
-D warnings,cargo fmt --check,cargo test, ruff, mypy clean.PYTHON_GIL=0: 566 passed, 1 skipped, 5 xfailed, three consecutive runs. Import withoutPYTHON_GILleaves the GIL disabled with warnings-as-errors.The macOS/Windows free-threaded wheel steps cannot be run locally; if
setup-pythonor maturin behaves differently there I'll fix it up from the CI logs.Related: #46 (abi3 would remove the free-threaded wheels on 3.14; decide together), #45 (PR 2, releasing the GIL around the parse, follows separately).
https://claude.ai/code/session_019WbvXZFm8Nb2LXF2kiWoWW
Generated by Claude Code