(3) poison-as-panic wrapper for shuttle::sync#1543
Closed
daniel-noland wants to merge 2 commits into
Closed
Conversation
8 tasks
daniel-noland
force-pushed
the
pr/daniel-noland/concurrency-stack/4-workspace-transition
branch
from
03fec49 to
8878b3c
Compare
daniel-noland
force-pushed
the
pr/daniel-noland/concurrency-stack/5-facade-shuttle
branch
from
d19d4e1 to
7470063
Compare
daniel-noland
force-pushed
the
pr/daniel-noland/concurrency-stack/4-workspace-transition
branch
from
8878b3c to
9632629
Compare
daniel-noland
force-pushed
the
pr/daniel-noland/concurrency-stack/5-facade-shuttle
branch
from
7470063 to
89896cd
Compare
Introduce a `parking_lot`-shaped facade over `Mutex` / `RwLock` and
rewire the data-path crates through it in a single atomic step.
Three pieces are bundled here because they can't be split without
leaving the workspace in a state where the facade and its consumers
disagree on whether `Mutex::lock` returns `LockResult<Guard>` or a
naked guard. Each piece on its own is internally coherent; together
they form the smallest unit that builds clean.
## std::sync wrapper (poison-as-panic)
New `concurrency::sync` module exposing `Mutex` / `RwLock` with a
`parking_lot`-shaped naked-guard surface backed by a poison-as-panic
wrapper around `std::sync`. Workspace policy treats a crashed thread
as a crashed process: poison surfacing inside a lock acquire panics
through `concurrency::sync::poisoned()` rather than handing back a
possibly-torn `LockResult`.
* One cold `#[inline(never)] fn poisoned() -> !` keeps the hot path
branch-free; the wrapper is a single `match` per acquire.
* `RwLock::upgradable_read` is implemented as exclusive `write()`
for now -- sound but lossy. Documented at the backend module
level.
* `Arc`, `Weak`, atomics, `Condvar`, `mpsc`, `OnceLock`, etc. are
re-exported from `std::sync` unchanged.
## parking_lot as the production default
Adds a `parking_lot` Cargo feature (enabled by default) and
`concurrency/src/sync/parking_lot_backend.rs` -- a zero-cost
re-export of `parking_lot::{Mutex, RwLock}` plus the std re-exports
for everything `parking_lot` doesn't ship.
* Backend routing in `concurrency/src/sync/mod.rs` picks
`parking_lot_backend` when `feature = "parking_lot"` is on, falls
back to `std_backend` otherwise.
* `_strict_provenance` feature overrides `parking_lot` and routes
through `std_backend`: `parking_lot_core::word_lock` uses
integer-to-pointer casts that `-Zmiri-strict-provenance` rejects,
and the CI miri job (`--features=_strict_provenance`) exercises
the fallback slot, which needs `std::sync` underneath.
* Workspace `Cargo.toml` gets a `tokio` parking_lot override entry
so the `tokio` parking_lot feature is enabled workspace-wide.
## Workspace transition
Sweep the data-path crates (`flow-entry`, `flow-filter`, `nat`,
`net`, `pipeline`, `routing`, `stats`) to consume
`dataplane_concurrency::sync::{Arc, Mutex, RwLock, atomic, ...}`
instead of `std::sync::*` / `parking_lot::*` directly.
* Call sites drop the `.unwrap()` / `.expect("poisoned")` noise the
`LockResult`-shaped surface forced, since the facade returns
naked guards. Same shape as the existing `parking_lot::Mutex`
consumers.
* `concurrency_mode(std)` annotations at QSBR-touching test sites
in `nat::stateful::apalloc::test_alloc`,
`routing::fib::test`, and `flow-entry::flow_table::table::tests`
keep the existing std-only feature gating intact.
* `nat::portfw::flow_state::get_packet_port_fw_state`
simplification: the explicit `drop(guard)` dance before `debug!`
calls is removed (per the recorded author intent that it isn't
worth the code complexity); the guard now drops implicitly at
the end of scope.
Mechanical-by-design -- the sweep is mostly `use` rewrites and
`.unwrap()` removal. The semantic change is one step: every
consumer now gets the facade-routed primitives, so flipping the
loom/shuttle features at the top of the workspace becomes
meaningful (subsequent PRs add the shuttle and loom backends).
Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Add a shuttle backend to the sync facade so existing code can run
unchanged under shuttle's random / PCT / DFS schedulers.
Backend selection in concurrency/src/sync/mod.rs now picks shuttle
ahead of parking_lot when any shuttle* feature is on (and loom is not).
loom still wins overall; the loom backend gets its own wrapper in the
final PR of this stack.
concurrency/src/sync/shuttle_backend.rs mirrors std_backend.rs: a thin
poison-as-panic wrap around shuttle::sync::{Mutex, RwLock} that returns
naked guards, plus re-exports for Arc / Weak / atomics / Condvar / mpsc
that pass through unchanged. OnceLock falls back to std::sync because
shuttle does not ship one (it is pure-std machinery and does not need
model-checker integration).
Three feature flags share one dep:shuttle so callers can pick a
scheduler without a second feature axis:
* `shuttle` -- random scheduler (default first-time choice).
* `shuttle_pct` -- bias toward rare interleavings.
* `shuttle_dfs` -- exhaustive small-state exploration.
The scheduler difference is purely runtime; the same wrapper compiles
for all three.
Cleanup as a side effect:
* quiescent.rs's cfg dispatch shrinks from
`any(feature = "loom", feature = "shuttle")` to `feature = "loom"`
because the wrapped shuttle backend now returns a naked guard.
* slot.rs's fallback branch picks up shuttle_pct/shuttle_dfs and
factors the loom-only unwrap into a local `unwrap_lock!` macro;
the macro disappears with the loom wrapper.
Tests:
* `quiescent_shuttle.rs` runs `check_random` and `check_pct` and now
enables on any of the three shuttle features; the std-only test
gates on `not(any(...))` so PCT/DFS don't accidentally pick up the
std code path.
* `quiescent_protocol.rs` / `quiescent_properties.rs` widen the
"std only" gate identically.
* `lib.rs`'s `pub use ...::thread` matches the same widened gate, so
threads spawned from shuttle test bodies use the shuttle executor
instead of falling through to `std::thread` and tripping shuttle's
`ExecutionState NotSet` panic.
Verified with `cargo nextest run -p dataplane-concurrency` under each
of: default features, `--features shuttle`, `--features shuttle_pct`,
`--features shuttle_dfs`.
Signed-off-by: Daniel Noland <daniel@githedgehog.com>
daniel-noland
force-pushed
the
pr/daniel-noland/concurrency-stack/5-facade-shuttle
branch
from
89896cd to
586483b
Compare
daniel-noland
force-pushed
the
pr/daniel-noland/concurrency-stack/4-workspace-transition
branch
from
9632629 to
2be0637
Compare
3 tasks
daniel-noland
changed the title
daniel-noland
changed the title
daniel-noland
force-pushed
the
pr/daniel-noland/concurrency-stack/4-workspace-transition
branch
2 times, most recently
from
1932e47 to
1ce8d26
Compare
Collaborator
Author
|
Subsumed by #1542. The shuttle backend (), the feature lattice ( |
Collaborator
Author
|
Subsumed by #1542. The shuttle backend ( Folded those pieces into #1542 so each PR in the stack is independently green. Stack is now 3 PRs:
|
daniel-noland
deleted the
pr/daniel-noland/concurrency-stack/5-facade-shuttle
branch
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
1 participant
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
concurrency/src/sync/shuttle_backend.rsmirrorsstd_backend.rs: a thin poison-as-panic wrap aroundshuttle::sync::{Mutex, RwLock}that returns naked guards, plus re-exports forArc/Weak/ atomics /Condvar/mpscthat pass through unchanged.OnceLockfalls back tostd::syncbecause shuttle doesn't ship one.dep:shuttle, arranged as a chain (shuttle_dfs→shuttle_pct→shuttle):shuttle— random scheduler (default first choice).shuttle_pct— PCT, biases toward rare interleavings.shuttle_dfs— DFS, exhaustive small-state exploration.Same backend, scheduler chosen at runtime by
concurrency::stress. The chain means a singlefeature = "shuttle"cfg check is true under every variant, so the routing insync/mod.rsand the macro gates collapse to single-feature checks (noany(shuttle, shuttle_pct, shuttle_dfs)chains).concurrency/src/sync/mod.rspicksshuttle_backendahead ofparking_lot_backendwhenfeature = "shuttle"is on.with_shuttle!/with_std!macro gates usefeature = "shuttle"directly — the chain makes this correct undershuttle_pctandshuttle_dfsbuilds too.lib.rs'sthreadre-exports gated identically so threads spawned from shuttle test bodies use the shuttle executor instead of falling through tostd::threadand trippingExecutionState NotSet.tests/quiescent_shuttle.rsandtests/scope_property.rsusefeature = "shuttle"directly for their backend gates.PR 3 of 4 in the concurrency facade stack. Depends on #1542 — facade + workspace sweep. The final PR adds the loom backend plus
#[concurrency::test]+ thethread::scopeshim.Test plan
cargo nextest run -p dataplane-concurrency --features shuttlepasses.cargo nextest run -p dataplane-concurrency --features shuttle_pctpasses.cargo nextest run -p dataplane-concurrency --features shuttle_dfspasses.cargo nextest run -p dataplane-concurrency(default) still passes.features/release/shuttlepasses.