diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3b0dc00..1c1369a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -24,7 +24,15 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - python-version: ["3.11", "3.12", "3.13", "3.14"] + # "3.14t" is the free-threaded build. The module declares that it runs + # without the GIL and free-threaded wheels are published, so the suite + # has to pass there too. + python-version: ["3.11", "3.12", "3.13", "3.14", "3.14t"] + env: + # Pin uv to the interpreter this job installs. The runner also carries + # system Pythons, and for the free-threaded entry uv would otherwise + # prefer a GIL build of the same version. + UV_PYTHON: ${{ matrix.python-version }} steps: - uses: actions/checkout@v7 @@ -48,8 +56,19 @@ jobs: - name: Run Rust tests run: cargo test --verbose - + - name: Run Python tests + if: ${{ !endsWith(matrix.python-version, 't') }} + run: uv run pytest --verbose --tb=short + + - name: Run Python tests (free-threaded, GIL disabled) + if: ${{ endsWith(matrix.python-version, 't') }} + # PYTHON_GIL=0 keeps the GIL off even if some dependency has not declared + # free-threading support, so the suite exercises the free-threaded path + # rather than a silently re-enabled GIL. tests/test_free_threading.py + # separately checks that importing this module alone does not re-enable it. + env: + PYTHON_GIL: "0" run: uv run pytest --verbose --tb=short lint: @@ -156,8 +175,10 @@ jobs: platform: - runner: windows-latest target: x64 + free-threaded: true - runner: windows-latest target: x86 + free-threaded: false steps: - uses: actions/checkout@v7 - uses: actions/setup-python@v7 @@ -170,6 +191,25 @@ jobs: target: ${{ matrix.platform.target }} args: --release --out dist --find-interpreter sccache: 'true' + # The manylinux images used by the Linux jobs carry python3.14t, so + # --find-interpreter builds the free-threaded wheel there on its own. This + # runner does not, so install the interpreter and build that wheel + # explicitly. Installed after the first build so --find-interpreter above + # does not also pick it up. + - name: Set up free-threaded Python + if: ${{ matrix.platform.free-threaded }} + id: free-threaded + uses: actions/setup-python@v7 + with: + python-version: '3.14t' + architecture: ${{ matrix.platform.target }} + - name: Build free-threaded wheel + if: ${{ matrix.platform.free-threaded }} + uses: PyO3/maturin-action@v1 + with: + target: ${{ matrix.platform.target }} + args: --release --out dist -i ${{ steps.free-threaded.outputs.python-path }} + sccache: 'true' - name: Upload wheels uses: actions/upload-artifact@v7 with: @@ -197,6 +237,19 @@ jobs: target: ${{ matrix.platform.target }} args: --release --out dist --find-interpreter sccache: 'true' + # See the Windows job: this runner has no python3.14t, so the free-threaded + # wheel needs the interpreter installed and an explicit build. + - name: Set up free-threaded Python + id: free-threaded + uses: actions/setup-python@v7 + with: + python-version: '3.14t' + - name: Build free-threaded wheel + uses: PyO3/maturin-action@v1 + with: + target: ${{ matrix.platform.target }} + args: --release --out dist -i ${{ steps.free-threaded.outputs.python-path }} + sccache: 'true' - name: Upload wheels uses: actions/upload-artifact@v7 with: diff --git a/CHANGELOG.md b/CHANGELOG.md index f62b4d8..f1464c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,35 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- **Free-threaded Python.** The test suite runs on the free-threaded build of + Python 3.14 (`python3.14t`), and free-threaded wheels ship for Linux, macOS and + Windows x64. Importing `cel` leaves the GIL disabled + ([#45](https://github.com/hardbyte/python-common-expression-language/issues/45)). + +### Changed + +- `Program` and `OptionalValue` are immutable; attributes can no longer be + assigned on them. +- One `Context` may be evaluated against from many threads at once, and + evaluations see a consistent snapshot. An evaluation that starts during + `add_variable`, `add_function` or `update` waits for it. Mutating a `Context` + from two threads at once raises `RuntimeError` on a free-threaded interpreter; + build a context before sharing it, or guard mutation with a lock. + +### Fixed + +- On a free-threaded interpreter, evaluating against a `Context` while another + thread was mutating it raised `ValueError: evaluation_context must be a Context + object or a dict`. It now waits for the mutation to finish. +- Loading the extension module a second time in one process (from a + sub-interpreter, for instance) no longer panics. + +### Updated + +- Building from the sdist requires maturin 1.14 or later. + ## [0.10.0] - 2026-09-15 Fixes `import cel` in a clean install, which has been broken since 0.6.0 for diff --git a/docs/reference/python-api.md b/docs/reference/python-api.md index cf00408..0e70f2c 100644 --- a/docs/reference/python-api.md +++ b/docs/reference/python-api.md @@ -213,6 +213,16 @@ reuses that work for every subsequent `evaluate()` or `Program.execute()` call until the context is modified. A dict passed as the context is converted afresh on every call, because it can change between calls without notice. +**Threads.** A `Program` is immutable and safe to share between threads. A +`Context` is safe to evaluate against from many threads at once: each evaluation +runs against a consistent snapshot, and a change made from another thread applies +from the next evaluation (an evaluation that starts while another thread is +mid-mutation briefly waits for it). Mutating one `Context` from several threads at the same +time is not supported; on a free-threaded interpreter (`python3.14t`) it raises +`RuntimeError: Already borrowed` rather than corrupting state. Build a context +before sharing it, or guard mutation with your own lock. Free-threaded wheels +(`cp314t`) are published and the module declares that it does not need the GIL. + ```python from cel import evaluate, Context diff --git a/pyproject.toml b/pyproject.toml index 66200db..7e2d28e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -42,7 +42,8 @@ Homepage = "https://github.com/hardbyte/python-common-expression-language" Repository = "https://github.com/hardbyte/python-common-expression-language" [build-system] -requires = ["maturin>=1.8,<2.0"] +# 1.14 is the first maturin that discovers free-threaded (3.14t+) interpreters. +requires = ["maturin>=1.14,<2.0"] build-backend = "maturin" diff --git a/src/context.rs b/src/context.rs index 387f94f..e5103f9 100644 --- a/src/context.rs +++ b/src/context.rs @@ -2,6 +2,7 @@ use ::cel::objects::TryIntoValue; use ::cel::{Context as CelContext, Value}; use pyo3::exceptions::PyValueError; use pyo3::prelude::*; +use pyo3::sync::MutexExt; use pyo3::types::PyDict; use pyo3::IntoPyObjectExt; use std::collections::HashMap; @@ -38,8 +39,14 @@ use std::sync::{Arc, Mutex, PoisonError}; /// ``add_function()`` or ``update()``. /// /// Thread Safety: -/// Context objects are not thread-safe. Create separate Context instances -/// for concurrent use or implement your own synchronization. +/// Evaluating against one Context from several threads at once is safe: +/// each evaluation uses a consistent snapshot of the variables and +/// functions, and a change made from another thread applies from the next +/// evaluation (an evaluation that starts mid-mutation briefly waits for +/// it). Mutating one Context from several threads at the same time is not +/// supported; on a free-threaded interpreter it raises +/// ``RuntimeError: Already borrowed`` rather than corrupting state. Build +/// the context before sharing it, or guard mutation with your own lock. /// /// Performance Tips: /// - Reuse Context objects for multiple evaluations when possible: the @@ -86,7 +93,14 @@ impl Context { /// even if a Python callback mutates this `Context` mid-evaluation; the /// mutation simply takes effect from the next evaluation. pub(crate) fn cel_context(&self, py: Python<'_>) -> Arc> { - let mut cached = self.cel.lock().unwrap_or_else(PoisonError::into_inner); + // `lock_py_attached` rather than `lock`: Python API runs under this lock + // (`clone_ref` in `build_cel_context`), and on a free-threaded + // interpreter a thread that blocks on a plain mutex while attached can + // stall a stop-the-world pause that the lock holder is waiting for. + let mut cached = self + .cel + .lock_py_attached(py) + .unwrap_or_else(PoisonError::into_inner); if let Some(existing) = cached.as_ref() { return Arc::clone(existing); } diff --git a/src/lib.rs b/src/lib.rs index 84897b1..b0c82c9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -42,6 +42,21 @@ pub(crate) fn new_environment() -> CelContext<'static> { CelContext::with_env(stdlib_env()) } +/// Compile-time proof that the cel-rust types this module shares between threads +/// are `Send + Sync`. Free-threaded Python loads this module without a GIL (see the +/// `gil_used` declaration on the module), so a `Context` cached in an `Arc` and a +/// `Program` held by several threads must be sound to share. cel-rust guarantees +/// this today (`Val`, `Function` and `VariableResolver` all require it); if an +/// upstream release drops the bound, this fails to build instead of letting a data +/// race into a `cp314t` wheel. +const _: fn() = || { + fn assert_send_sync() {} + assert_send_sync::(); + assert_send_sync::>(); + assert_send_sync::(); + assert_send_sync::(); +}; + /// A compiled CEL program that can be executed multiple times with different contexts. /// /// This is useful when you need to evaluate the same expression many times with different @@ -60,7 +75,9 @@ pub(crate) fn new_environment() -> CelContext<'static> { /// result1 = program.execute({"price": 10, "quantity": 20}) # True /// result2 = program.execute({"price": 5, "quantity": 10}) # False /// ``` -#[pyclass(name = "Program")] +// `frozen`: a compiled program never changes after construction, so Python may share +// it between threads without PyO3's per-access borrow tracking. +#[pyclass(name = "Program", frozen)] struct PyProgram { program: Program, source: String, @@ -152,7 +169,7 @@ impl PyProgram { } /// A CEL optional value wrapper for Python. -#[pyclass(name = "OptionalValue")] +#[pyclass(name = "OptionalValue", frozen)] struct PyOptionalValue { value: Option, } @@ -548,6 +565,31 @@ struct Environment { resolver: Option, } +/// Takes the shared borrow of a `Context` that an evaluation needs. +/// +/// On a free-threaded interpreter another thread may be inside a mutator +/// (`add_variable`, `update`, ...) at this moment, holding the exclusive borrow. +/// A mutator holds it only for the duration of one call, so yield a few times +/// before giving up, which lets readers ride out a concurrent update. If it +/// still cannot be borrowed, report that rather than the misleading "must be a +/// Context or a dict" that a failed `extract` would otherwise fall through to. +fn borrow_context<'py>( + bound: &Bound<'py, context::Context>, +) -> PyResult> { + const ATTEMPTS: usize = 64; + for _ in 0..ATTEMPTS { + match bound.try_borrow() { + Ok(context) => return Ok(context), + Err(_) => std::thread::yield_now(), + } + } + Err(PyRuntimeError::new_err( + "Context is being modified by another thread (already mutably borrowed). \ + Finish building a Context before sharing it between threads, or guard \ + mutation with a lock.", + )) +} + /// Turns the `evaluation_context` argument of `evaluate()`/`Program.execute()` /// into an [`Environment`], so the two entry points behave identically. fn prepare_environment(evaluation_context: Option<&Bound<'_, PyAny>>) -> PyResult { @@ -559,10 +601,11 @@ fn prepare_environment(evaluation_context: Option<&Bound<'_, PyAny>>) -> PyResul }; let py = evaluation_context.py(); - if let Ok(py_context) = evaluation_context.extract::>() { + if let Ok(bound_context) = evaluation_context.cast::() { // The borrow of the Python object ends when `py_context` drops at the end // of this block, before any Python callback can run, so a callback that // mutates the Context mid-evaluation does not hit a "borrowed" error. + let py_context = borrow_context(bound_context)?; let resolver = py_context .resolver .as_ref() @@ -1039,9 +1082,18 @@ fn execute_compiled_program( }) } -#[pymodule] +// `gil_used = false` has been PyO3's default since 0.28; spelling it out records the +// decision. Every pyclass here is `Send + Sync` (PyO3 asserts that at compile time), +// the shared standard-library `Env` is immutable behind a `LazyLock`, and the +// `Context` environment cache sits behind a `Mutex`, so a free-threaded interpreter +// may load this module without re-enabling the GIL. CI runs the test suite on +// `python3.14t` with `PYTHON_GIL=0` to keep that true. +#[pymodule(gil_used = false)] fn cel(_py: Python<'_>, m: &Bound<'_, PyModule>) -> PyResult<()> { - pyo3_log::init(); + // The module can be initialised more than once in one process (a + // sub-interpreter, for instance). `init()` panics when a logger is already + // installed; a second installation failing is harmless, so tolerate it. + let _ = pyo3_log::try_init(); m.add_function(wrap_pyfunction!(evaluate, m)?)?; m.add_function(wrap_pyfunction!(compile, m)?)?; diff --git a/tests/test_free_threading.py b/tests/test_free_threading.py new file mode 100644 index 0000000..e7339d2 --- /dev/null +++ b/tests/test_free_threading.py @@ -0,0 +1,122 @@ +"""Free-threaded CPython (``python3.14t``) support. + +The extension declares that it does not need the GIL (PyO3's ``gil_used = false``) +and free-threaded wheels are published, so these tests pin the two things that +declaration commits us to: importing the module must not make CPython re-enable +the GIL, and sharing objects between threads must never corrupt state. Most of +the file also runs on a regular build, where it documents the same contract. +""" + +import os +import subprocess +import sys +import sysconfig +from concurrent.futures import ThreadPoolExecutor + +import cel +import pytest + +FREE_THREADED_BUILD = bool(sysconfig.get_config_var("Py_GIL_DISABLED")) + +requires_free_threaded = pytest.mark.skipif( + not FREE_THREADED_BUILD, reason="only meaningful on a free-threaded CPython build" +) + + +@requires_free_threaded +def test_suite_runs_with_the_gil_disabled(): + """Sanity check for CI: the free-threaded leg really is exercising the GIL-free path. + + CI sets ``PYTHON_GIL=0`` for that leg so a dependency lacking the free-threading + declaration cannot quietly turn the GIL back on for the whole run. + """ + assert not sys._is_gil_enabled() + + +@requires_free_threaded +def test_importing_cel_does_not_reenable_the_gil(): + """Without ``PYTHON_GIL`` forcing the matter, importing ``cel`` alone must leave the GIL off. + + A module that has not declared free-threading support makes CPython re-enable + the GIL at import and emit a RuntimeWarning naming the module. Run in a fresh + interpreter so nothing else imported by the test session can mask the result, + and turn warnings into errors so the RuntimeWarning is loud. + """ + env = {k: v for k, v in os.environ.items() if k != "PYTHON_GIL"} + script = ( + "import sys, warnings\n" + "warnings.simplefilter('error')\n" + "import cel\n" + "assert cel.evaluate('1 + 1') == 2\n" + "assert not sys._is_gil_enabled(), 'importing cel re-enabled the GIL'\n" + ) + subprocess.run([sys.executable, "-c", script], env=env, check=True, timeout=60) + + +def test_program_is_immutable(): + """``Program`` is a frozen class, so sharing one between threads needs no locking.""" + program = cel.compile("x + 1") + with pytest.raises(AttributeError): + program.source = "y" # type: ignore[misc] + with pytest.raises(AttributeError): + program.anything = 1 # type: ignore[attr-defined] + + +def test_optional_value_is_immutable(): + opt = cel.OptionalValue.of(1) + with pytest.raises(AttributeError): + opt.anything = 1 # type: ignore[attr-defined] + + +def test_concurrent_mutation_raises_rather_than_races(): + """Mutating one ``Context`` from several threads is unsupported but never unsafe. + + Readers see a consistent snapshot on every evaluation and briefly wait out a + writer that is mid-mutation. A writer that collides with another borrow of the + same ``Context`` gets PyO3's ``RuntimeError: Already borrowed`` (only reachable + on a free-threaded build); a reader that cannot get the borrow at all gets a + ``RuntimeError`` naming the concurrent modification. Neither ever corrupts the + context: whatever interleaving happens, every value observed must be one that + some writer actually stored. + """ + ctx = cel.Context({"v": 0}) + program = cel.compile("v") + rounds = 500 + + def writer(_): + for n in range(rounds): + try: + ctx.add_variable("v", n) + except RuntimeError as exc: + assert "borrowed" in str(exc).lower(), exc + + def reader(_): + for _ in range(rounds): + try: + result = program.execute(ctx) + except RuntimeError as exc: + assert "borrowed" in str(exc).lower(), exc + continue + assert isinstance(result, int) and 0 <= result < rounds, result + + with ThreadPoolExecutor(max_workers=8) as pool: + futures = [pool.submit(writer if i % 2 else reader, i) for i in range(8)] + for future in futures: + future.result() + + assert 0 <= ctx.variables["v"] < rounds + + +def test_shared_program_and_context_across_threads_agree(): + """Many threads evaluating one frozen ``Program`` against one ``Context`` all get the same answer.""" + ctx = cel.Context({"items": list(range(200))}) + ctx.add_function("bump", lambda n: n + 1) + program = cel.compile("bump(items.filter(i, i % 2 == 0).size())") + + def work(_): + return [program.execute(ctx) for _ in range(200)] + + with ThreadPoolExecutor(max_workers=8) as pool: + results = list(pool.map(work, range(8))) + + assert all(result == [101] * 200 for result in results)