(3) #[concurrency::test] macro and thread::scope shim#1544
Draft
daniel-noland wants to merge 2 commits into
Draft
Conversation
1ce8d26 to
8d86891
Compare
d89cf27 to
6939124
Compare
8d86891 to
0364eb1
Compare
This was referenced May 15, 2026
6939124 to
732e640
Compare
Ship the test-time infrastructure that lets a single source file run
under every backend that the facade (in the prior PR) routes to.
Three pieces:
* `concurrency::stress(body)` -- runtime dispatcher selecting
`loom::model` / `shuttle::check_random` / `_pct` / `_dfs` /
direct call based on the active feature.
* `#[concurrency::test]` -- attribute proc-macro expanding to
`#[test]` + `stress(|| body)`. One source file, five execution
strategies, no `loom::model(|| { ... })` boilerplate at call
sites.
* `concurrency::thread::scope` -- works on every backend. Std and
shuttle re-export theirs; loom 0.7 has no `scope`, so this PR
ships a local shim built on `loom::spawn` plus an
`Arc<Mutex<Option<T>>>` keepalive that preserves the same
drop-affinity guarantee `std::thread::scope` gives.
## stress + #[concurrency::test]
`concurrency::stress(body)`:
* default -- `body()` direct, no exploration
* `loom` -- `loom::model(body)`
* `shuttle` -- `shuttle::check_random(body, 16)`
* `shuttle_pct` -- `shuttle::check_pct(body, 16, 3)`
* `shuttle_dfs` -- `shuttle::check_dfs(body, Some(16))`
The dispatch carries a defensive `_ => compile_error!` arm: if a
new model-checker feature is added to the exclusion list but no
matching dispatch arm is wired up, the build fails loudly rather
than silently skipping exploration.
`#[concurrency::test]` is a new attribute proc-macro in
`concurrency-macros`. Under model-checker backends it emits a
nested `mod <fn_name> { mod concurrency_model { #[test] fn
<backend>() { ... } } }` so test reports / JUnit show the active
backend as the leaf name (e.g.
`snapshot_observes::concurrency_model::loom`). Default backend
stays flat. `#[should_panic]` / `#[ignore]` thread through;
`async fn` and `fn(args)` are rejected at parse time.
The crate path is resolved at expansion time via
`proc-macro-crate`, so consumers depending on
`dataplane-concurrency` under any alias work without setup;
integration tests inside this crate use `extern crate
dataplane_concurrency as concurrency;` since cargo rejects
self-deps.
`stress` is re-exported at the crate root for the macro expansion
but hidden from rustdoc (`#[doc(hidden)]`) -- users land on the
macro.
## thread::scope shim
`concurrency::thread` becomes a module (was a `pub use` alias) and
re-exports the active backend's `thread::*`. Under loom it also
includes a local `thread::scope` shim built on `loom::spawn` plus
an `Arc<Mutex<Option<T>>>` keepalive pattern. A narrow
`mem::transmute` lifts the spawned closure's `'scope` to
`'static` (loom 0.7 has no `spawn_unchecked`); the keepalive
trait object keeps its honest `'scope` and `ScopeInner<'scope>`
lives in `ManuallyDrop` so dropck does not constrain `'scope`
past `scope()`'s body. Per-site SAFETY comments at both the
transmute and the `ManuallyDrop::drop`.
Drop-affinity is enforced by an explicit `take_payload` on main
during teardown, not by an Arc-count assertion. Loom's
`JoinHandle::join` synchronises only on `notify`, which can fire
before the spawned thread's wrapper has finished dropping its
captures -- so the count-based check fires spuriously in some
schedules. Manual `take` on main gives the same guarantee
unconditionally.
## quiescent.rs / slot.rs cleanup
With every backend now returning naked guards from `.lock()` (the
prior PR), the remaining `LockResult` unwrap dispatch in
`quiescent.rs` disappears; `.lock()` calls reduce to plain
naked-guard form across all backends.
## Tests
* `tests/quiescent_model.rs` replaces `tests/quiescent_loom.rs`.
Each test is now a plain function annotated with
`#[concurrency::test]`; the body uses `thread::scope` instead
of the `Box::into_raw` + `&'static` lifetime workaround the
old loom-only file needed. Single source runs under any
backend, including the four-way schedule exploration shuttle
offers.
* `tests/thread_scope.rs` -- contract tests for `thread::scope`
(single/multi-spawn join, lifetime borrows, drop affinity,
nested-scope re-entry). Pins shim regressions at the right
layer.
* `tests/scope_property.rs` -- bolero x shuttle property test
generating random spawn-count / per-spawn-ops plans; pins the
"scope conservation" invariant (counter at scope return
equals the sum of generated increments) across many shapes
and interleavings.
* `tests/stress_dispatch.rs` -- coarse dispatch-routing check:
the default backend invokes the body exactly once; the
model-check backends invoke it more than once. Plus
`#[should_panic]` / `#[ignore]` attribute pass-through.
* `tests/arc_weak.rs` -- contract tests for `Arc` / `Weak` that
pass under every backend, plus loom-gated tests pinning the
documented shim quirks. File-level
`#![cfg(not(feature = "shuttle_pct"))]` -- PCT cannot run
single-threaded protocol checks.
## Documentation
`lib.rs` gains a "Compiles under loom != exhaustively checked
under loom" section listing each documented shim limitation (Weak
strong-clone semantics, `weak_count` panic, RwLock
upgradable_read, OnceLock ordering invisibility, Mutex::new const
divergence) with the workspace consequence -- most notably calling
out NAT's `Weak::upgrade().is_none()` patterns and why NAT is not
in the loom matrix today.
`sync/mod.rs` gains a "Portability footguns the facade does not
paper over" section.
`justfile` nextest filter: empty under loom (the archived
binaries are already feature-filtered), `shuttle` substring under
shuttle (matches both `quiescent_shuttle` and the new
`concurrency_model::shuttle` leaves).
Verified with `cargo nextest run -p dataplane-concurrency` under
each of: default features, `--features shuttle`, `--features
shuttle_pct`, `--features shuttle_dfs`, and `--features loom
--release`. Also verified clippy passes under each backend and
under `--all-features` (the silence_clippy escape hatch routes
through `std` and the new thread / stress modules play nice with
that).
Signed-off-by: Daniel Noland <daniel@githedgehog.com>
8f42074 to
5e2b328
Compare
Replace the two-entry `features` matrix (loom + shuttle, single
schedule each) with a single `concurrency` job whose four steps each
exercise a different scheduler:
* shuttle -- random
* shuttle_pct -- PCT (rare interleavings)
* shuttle_dfs -- DFS (exhaustive small-state)
* loom -- the loom backend; scoped to `concurrency` only
because workspace feature unification under
`feature = "loom"` breaks crates that consume
`Weak<T>` / `Arc::downgrade` from
`concurrency::sync` (loom 0.7 doesn't ship
`Weak`).
Named steps mean GitHub's check list shows the failing scheduler
directly (`concurrency / shuttle_pct`) instead of one collapsed
`features/release/shuttle` line per backend. One runner does the
nix-setup once and reuses the cargo cache across all four steps;
the matrix shape spun up two runners and paid that cost twice.
`needs: check` is added so model-checker time is only burned after
clippy / build is green on the same SHA.
Summary job's stale `needs.features` reference is renamed to
`needs.concurrency` (without the rename it always emitted
`::error:: Some features job(s) failed` because `needs.features`
resolved to `null`).
The orphan `&release-build` YAML anchor (its only consumer was the
deleted matrix) is dropped; the matrix entry it labelled stays.
The new `concurrency` job needs its own `env:` rather than reusing
the `*check-env` anchor, because that anchor's `JUST_VARS`
references `matrix.build.*` and this job has no matrix. Each step
inlines the full `JUST_VARS` (with `docker_sock` / `debug_justfile`
/ `oci_repo`) -- the previous shape would have silently dropped
those settings via the step-level `JUST_VARS` override, falling
back to justfile defaults that point at local docker / a localhost
registry.
`justfile`'s nextest filter widens from exact-match
(`features == "shuttle"`) to a regex match (`features =~ "^shuttle"`)
so the `shuttle_pct` and `shuttle_dfs` steps also apply the
`shuttle` substring filter. Without it, every workspace test runs
under the shuttle feature; non-shuttle tests panic with
"ExecutionState NotSet" because `concurrency::sync` types ARE
shuttle primitives once the feature is on, and touching them
outside a `shuttle::check_*`-wrapped body trips the executor.
Signed-off-by: Daniel Noland <daniel@githedgehog.com>
5e2b328 to
08e23f9
Compare
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.
Summary
The capstone PR. Ship the test-time infrastructure that lets a single source file run under every backend the facade (in #1542) routes to.
Three pieces, plus the test suite that exercises them.
concurrency::stress+#[concurrency::test]concurrency::stress(body)dispatches toloom::model/shuttle::check_random/_pct/_dfs/ direct call based on the active feature. The dispatch has a defensive_ => compile_error!arm: if a new model-checker feature is added to the exclusion list but no matching dispatch arm is wired up, the build fails loudly rather than silently skipping exploration.#[concurrency::test]proc-macro expands to#[test]plusstress(|| body). Under model-checker backends it emits a nestedmod <fn_name> { mod concurrency_model { #[test] fn <backend>() { ... } } }so test reports / JUnit show the active backend as the leaf name (e.g.snapshot_observes::concurrency_model::loom). Default backend stays flat.#[should_panic]/#[ignore]thread through;async fnandfn(args)are rejected at parse time.proc-macro-crate, so consumers under any alias work without setup; integration tests inside this crate useextern crate dataplane_concurrency as concurrency;since cargo rejects self-deps.stressis re-exported at the crate root for the macro expansion but hidden from rustdoc (#[doc(hidden)]) -- users land on the macro.thread::scopeshimconcurrency::thread::scopeworks on every backend. Std and shuttle re-export theirs; loom 0.7 doesn't havescope, so we shipconcurrency/src/thread/loom_scope.rs: built onloom::spawnplus anArc<Mutex<Option<T>>>keepalive that preserves the same drop-affinity guaranteestd::thread::scopegives.take_payloadon main during teardown, not by an Arc-count assertion. Loom'sJoinHandle::joinsynchronises only onnotify, which can fire before the spawned thread's wrapper has finished dropping its captures -- so the count-based check fires spuriously in some schedules. Manualtakeon main gives the same guarantee unconditionally.mem::transmuteto lift the spawned closure's'scopeto'static(loom 0.7 has nospawn_unchecked); the keepalive trait object keeps its honest'scopeandScopeInner<'scope>lives inManuallyDropso dropck doesn't constrain'scopepastscope()'s body. Per-site SAFETY comments at both the transmute and theManuallyDrop::drop.Tests
tests/quiescent_model.rsreplaces the loom-onlyquiescent_loom.rs. One source, every backend, via#[concurrency::test]+thread::scope.tests/thread_scope.rs-- contract tests forthread::scope(single/multi-spawn, drop affinity, nested-scope re-entry).tests/scope_property.rs-- bolero x shuttle conservation property over generated spawn/op plans.tests/stress_dispatch.rs--stressinvokes body once on default, more than once on model-check backends; plus#[should_panic]/#[ignore]attribute pass-through.tests/arc_weak.rs--Arc/Weakcontract (passes under every backend, plus loom-only quirk tests). File-level#![cfg(not(feature = "shuttle_pct"))]-- PCT can't run single-threaded protocol checks.justfilenextest filter: empty under loom (the archived binaries are already feature-filtered),shuttlesubstring under shuttle (matches bothquiescent_shuttleand the newconcurrency_model::shuttleleaves).Documentation
lib.rsgains a "Compiles under loom != exhaustively checked under loom" section listing each documented shim limitation (Weak strong-clone semantics,weak_countpanic, RwLock upgradable_read, OnceLock ordering invisibility, Mutex::new const divergence) with the workspace consequence -- most notably calling out NAT'sWeak::upgrade().is_none()patterns and why NAT isn't in the loom matrix today.sync/mod.rsgains a "Portability footguns the facade does not paper over" section.PR 3 of 3 in the concurrency facade stack. Depends on #1542 -- sync facade + backends + workspace sweep. After this lands, the stack reaches feature parity with the original PR.
Test plan
cargo nextest run -p dataplane-concurrency(default).cargo nextest run -p dataplane-concurrency --features shuttle.cargo nextest run -p dataplane-concurrency --features shuttle_pct.cargo nextest run -p dataplane-concurrency --features shuttle_dfs.cargo nextest run -p dataplane-concurrency --features loom --release.cargo clippy -p dataplane-concurrency --all-targets --all-features(silence_clippy path).features/release/loomandfeatures/release/shuttlepass.check/miri/powerpc64(the_strict_provenancejob) still passes.