Skip to content

feat(concurrency): free-threaded CPython support — memory-ordering fixes, gil_used=false, CI lane (LAB-511) - #265

Open
27Bslash6 wants to merge 2 commits into
mainfrom
agent/winston/e1532665
Open

feat(concurrency): free-threaded CPython support — memory-ordering fixes, gil_used=false, CI lane (LAB-511)#265
27Bslash6 wants to merge 2 commits into
mainfrom
agent/winston/e1532665

Conversation

@27Bslash6

Copy link
Copy Markdown
Contributor

Closes LAB-511.

Makes cachekit-py provably race-free under free-threaded CPython, with a CI lane that gates regressions. No wire/format change; crypto surfaces untouched.

The defect (LAB-506 panel, comment b80e0145)

decorators/session.py::_ensure_session_initialized published three module globals and relied on assignment order for its lock-free fast path — a guarantee only the GIL provides. A GIL-free reader observing pid+id before _session_start_ms sailed into get_session_start_ms()'s "should never happen" RuntimeError, which backend.py catches and turns into silently dropped session headers — the exact telemetry loss LAB-506 eliminated, resurfacing GIL-free. The fast path and in-lock double-check now gate on every published field. Regression tests pin the mid-publish state deterministically (fail pre-fix, pass post-fix) and an 8-thread hammer races first-touch init for real on the free-threaded lane.

What the free-threaded lane surfaced

Running the suites on 3.14t found a second real bug: AsyncMetricsCollector.flush() polled Queue.empty(), which flips at dequeue — before processing finishes. Routine flake under true concurrency, invisible under the GIL. Now waits on unfinished_tasks (zeroed by task_done() after processing).

Changes

  • Memory-ordering guarddecorators/session.py full-field gate; regression tests in test_saas_observability.py::TestMidPublishMemoryOrdering + test_free_threading.py.
  • Concurrency audit — every lock-free fast path / shared mutable state named by the ticket, documented with verdicts in docs/free-threading.md (stats registry + fork handlers, _FunctionStats, session identity, header cache, L1/L2 SWR single-flight, ObjectCache, _cached_keys, metrics singleton, Rust extension).
  • PyO3#[pymodule(gil_used = false)] (the 0.28+ default made explicit); justified by audit: &self-only pyclasses, AtomicU64 nonce, Mutex metrics, Send + Sync compiler-enforced.
  • CI safety nettest-freethreaded job: uv sync --python 3.14t --no-default-groups --group test --no-install-package hiredis (hiredis has no Py_mod_gil declaration; redis-py falls back to its pure-Python parser), asserts the GIL stays disabled after importing cachekit, runs unit + critical. Wired into ci-success. An autouse session fixture re-asserts GIL state at teardown in every xdist worker.
  • Dependency groupstest (free-threading-compatible core toolchain) split out of dev (which includes it via include-group); dev resolves identically to before.
  • Test guardspytest.importorskip so the core suites honestly run without the [data]/[json] extras; narrowed to per-test scope where a module-level skip would have dropped extra-free coverage (encryption invariants, protocol compliance).
  • Docsdocs/free-threading.md (support status, audit table, deferral), README thread-safety section.

Deferred (per acceptance criteria, explicitly)

Free-threaded wheels / declared support blocked on upstream: orjson (build script rejects free-threaded interpreters, no FT wheels through 3.12.0), hiredis (no Py_mod_gil), numpy/pandas/pyarrow ([data] extra coverage incomplete). When they clear: add -i python3.14t to the build-wheels matrix.

Verification

  • 3.14t (GIL verified disabled): unit 1681 passed / 53 skipped, critical 233 passed — zero free-threading failures after the two fixes.
  • 3.13 GIL build (no regression): unit 1951 passed, critical 234 passed, ruff + basedpyright + cargo fmt/clippy + markdown-docs (121) green.
  • Expert panel (high stakes): security & pragmatism ships-as-is; bug-hunter MAJ (per-worker GIL assertion) and craftsman 2×MAJ+MIN applied in the second commit; one cut rejected (the four mid-publish tests pin four distinct call paths).

…xes, gil_used=false, CI lane (LAB-511)

Make cachekit-py provably race-free under free-threaded CPython and gate
regressions in CI:

- decorators/session.py: the lock-free fast path and in-lock double-check
  now gate on every published field (_session_start_ms included). Assignment
  order only guaranteed visibility order under the GIL; a GIL-free reader
  observing pid+id before start_ms hit the 'should never happen'
  RuntimeError and silently dropped session headers (the LAB-506 telemetry
  loss, resurfacing GIL-free). Regression tests pin the mid-publish state
  deterministically and fail pre-fix.
- reliability/metrics_collection.py: AsyncMetricsCollector.flush polled
  Queue.empty(), which flips at dequeue — before processing finishes. Waits
  on unfinished_tasks now. Was a routine flake on the free-threaded lane.
- rust/src/lib.rs: #[pymodule(gil_used = false)] — the PyO3 0.28+ default
  made explicit, justified by the LAB-511 audit (AtomicU64 nonce, Mutex
  metrics, &self-only pyclasses, Send+Sync compiler-enforced).
- CI: new test-freethreaded job runs unit+critical on 3.14t, asserts the
  GIL stays disabled after importing cachekit (hiredis excluded — no
  Py_mod_gil declaration; redis-py falls back to pure-Python parser).
- pyproject: dependency-groups split into test (free-threading-compatible
  core toolchain) + dev (includes test; adds the extras without
  free-threaded wheels: orjson, numpy, pandas, pyarrow).
- tests: importorskip guards so the core suites honestly run without the
  [data]/[json] extras; full audit table in docs/free-threading.md.

Free-threaded wheels/classifiers explicitly deferred: orjson (build rejects
free-threaded), hiredis (no Py_mod_gil), numpy/pandas/pyarrow coverage.
…p encryption/protocol suites (LAB-511)

Panel findings applied:
- tests/conftest.py: autouse session-scoped fixture fails the run if the
  GIL got re-enabled on a free-threaded build — runs in EVERY xdist worker
  and both suites, covering lazily-imported extensions the single-process
  CI pre-flight and one-worker in-suite check missed (bug-hunter MAJ).
- test_encryption_security_invariants.py / test_serializer_protocol.py:
  module-level importorskip narrowed to the 2+1 tests that actually need
  orjson/pyarrow — the encryption invariants and protocol-compliance
  suites now run on the free-threaded lane (craftsman MAJ x2; +35 tests).
- README/docs: support claim narrowed to 3.14t — the only build the lane
  runs (craftsman MIN).

Rejected: cutting test_session_headers_present_mid_publish as duplicate —
it pins the get_session_headers fallback branch; the end-to-end test pins
the info.session_id branch. Distinct paths, both stay.
@coderabbitai

coderabbitai Bot commented Aug 30, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 28 minutes.

View limit details

Limit details: You’ve used all 6 included reviews currently available. Your 40 included PR review attempts over the past 7 days set your current allowance at 6 reviews per hour.

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 12584a8d-7000-4328-b341-dbe8bc2e9132

📥 Commits

Reviewing files that changed from the base of the PR and between e1b05ce and 23734a5.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (28)
  • .github/workflows/ci.yml
  • README.md
  • docs/README.md
  • docs/free-threading.md
  • pyproject.toml
  • rust/src/lib.rs
  • src/cachekit/decorators/session.py
  • src/cachekit/reliability/metrics_collection.py
  • tests/conftest.py
  • tests/critical/test_cache_serializer_compression.py
  • tests/critical/test_cache_serializer_patterns.py
  • tests/critical/test_encryption_integration.py
  • tests/critical/test_production_data_patterns.py
  • tests/unit/test_arrow_serializer.py
  • tests/unit/test_auto_serializer_mutation_and_corruption.py
  • tests/unit/test_auto_serializer_new_types.py
  • tests/unit/test_auto_serializer_numpy_integrity.py
  • tests/unit/test_docs_conftest_no_key_leak.py
  • tests/unit/test_encryption_security_invariants.py
  • tests/unit/test_free_threading.py
  • tests/unit/test_key_generator_blake2b.py
  • tests/unit/test_mmap_read_path.py
  • tests/unit/test_orjson_serializer.py
  • tests/unit/test_saas_observability.py
  • tests/unit/test_serializer_integrity.py
  • tests/unit/test_serializer_lazy_loading.py
  • tests/unit/test_serializer_protocol.py
  • tests/unit/test_xxhash_integrity.py

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Aug 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/cachekit/decorators/session.py 50.00% 0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant