Skip to content

(3) #[concurrency::test] macro and thread::scope shim#1544

Draft
daniel-noland wants to merge 2 commits into
pr/daniel-noland/concurrency-stack/4-workspace-transitionfrom
pr/daniel-noland/concurrency-stack/6-facade-loom
Draft

(3) #[concurrency::test] macro and thread::scope shim#1544
daniel-noland wants to merge 2 commits into
pr/daniel-noland/concurrency-stack/4-workspace-transitionfrom
pr/daniel-noland/concurrency-stack/6-facade-loom

Conversation

@daniel-noland
Copy link
Copy Markdown
Collaborator

@daniel-noland daniel-noland commented May 15, 2026

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 to loom::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] plus stress(|| body). 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.
  • Crate path is resolved at expansion time via proc-macro-crate, so consumers 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::scope works on every backend. Std and shuttle re-export theirs; loom 0.7 doesn't have scope, so we ship concurrency/src/thread/loom_scope.rs: built on loom::spawn plus an Arc<Mutex<Option<T>>> keepalive that preserves the same drop-affinity guarantee std::thread::scope gives.
  • 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.
  • One mem::transmute to lift 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 doesn't constrain 'scope past scope()'s body. Per-site SAFETY comments at both the transmute and the ManuallyDrop::drop.

Tests

  • tests/quiescent_model.rs replaces the loom-only quiescent_loom.rs. One source, every backend, via #[concurrency::test] + thread::scope.
  • tests/thread_scope.rs -- contract tests for thread::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 -- stress invokes body once on default, more than once on model-check backends; plus #[should_panic] / #[ignore] attribute pass-through.
  • tests/arc_weak.rs -- Arc / Weak contract (passes under every backend, plus loom-only quirk tests). File-level #![cfg(not(feature = "shuttle_pct"))] -- PCT can't run single-threaded protocol checks.
  • 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).

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 isn't in the loom matrix today.
  • sync/mod.rs gains 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).
  • CI features/release/loom and features/release/shuttle pass.
  • CI check/miri/powerpc64 (the _strict_provenance job) still passes.

@daniel-noland daniel-noland changed the base branch from main to pr/daniel-noland/concurrency-stack/4-workspace-transition May 15, 2026 22:59
@daniel-noland daniel-noland force-pushed the pr/daniel-noland/concurrency-stack/4-workspace-transition branch from 1ce8d26 to 8d86891 Compare May 15, 2026 23:02
@daniel-noland daniel-noland force-pushed the pr/daniel-noland/concurrency-stack/6-facade-loom branch 2 times, most recently from d89cf27 to 6939124 Compare May 15, 2026 23:39
@daniel-noland daniel-noland force-pushed the pr/daniel-noland/concurrency-stack/4-workspace-transition branch from 8d86891 to 0364eb1 Compare May 15, 2026 23:39
@daniel-noland daniel-noland changed the title (3) facade loom (3) #[concurrency::test] macro, stress dispatcher, thread::scope shim May 15, 2026
@daniel-noland daniel-noland force-pushed the pr/daniel-noland/concurrency-stack/6-facade-loom branch from 6939124 to 732e640 Compare May 15, 2026 23:47
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>
@daniel-noland daniel-noland added ci:+vlab Enable VLAB tests ci:+cross run cross compile jobs labels May 15, 2026
@daniel-noland daniel-noland changed the title (3) #[concurrency::test] macro, stress dispatcher, thread::scope shim (3) #[concurrency::test] macro and thread::scope shim May 15, 2026
@daniel-noland daniel-noland force-pushed the pr/daniel-noland/concurrency-stack/6-facade-loom branch 2 times, most recently from 8f42074 to 5e2b328 Compare May 16, 2026 00:06
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>
@daniel-noland daniel-noland force-pushed the pr/daniel-noland/concurrency-stack/6-facade-loom branch from 5e2b328 to 08e23f9 Compare May 16, 2026 00:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:+cross run cross compile jobs ci:+vlab Enable VLAB tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant