diff --git a/Cargo.lock b/Cargo.lock index 586ce4a4b..67f6ae04b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1236,6 +1236,7 @@ dependencies = [ "bolero", "dataplane-concurrency-macros", "loom", + "parking_lot", "shuttle", "static_assertions", ] @@ -5629,6 +5630,7 @@ dependencies = [ "bytes", "libc 0.2.186", "mio", + "parking_lot", "pin-project-lite", "signal-hook-registry", "socket2", diff --git a/Cargo.toml b/Cargo.toml index 975e34099..caafdd319 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -53,6 +53,9 @@ repository = "https://github.com/githedgehog/dataplane/" # # 1. correctly documenting what each package actually depends on, # 2. allowing builds under different environments (e.g. cross-compilation, wasm, miri, and so on). +# +# Exceptions: see `tokio = { ..., features = ["parking_lot"] }` below. Each exception has a comment +# justifying why it is workspace-wide. # Internal args = { path = "./args", package = "dataplane-args", features = [] } @@ -183,7 +186,13 @@ strum_macros = { version = "0.28.0", default-features = false, features = [] } syn = { version = "2.0.117", default-features = false, features = [] } thiserror = { version = "2.0.18", default-features = false, features = [] } thread_local = { version = "1.1.9", default-features = false, features = [] } -tokio = { version = "1.52.3", default-features = false, features = [] } +# Exception to the "no workspace-wide features" rule above. `tokio/parking_lot` is +# not an additive feature in the usual sense -- it picks the lock implementation +# tokio uses internally for its runtime. Production builds already pull +# parking_lot in via `concurrency::sync`, so the only thing scoping this +# per-crate would buy is divergent runtime behaviour between test binaries +# and the real dataplane. Keep it global. +tokio = { version = "1.52.3", default-features = false, features = ["parking_lot"] } tokio-util = { version = "0.7.18", default-features = false, features = [] } tonic = { version = "0.14.6", default-features = false, features = [] } tracing = { version = "0.1.44", default-features = false, features = [] } diff --git a/concurrency/Cargo.toml b/concurrency/Cargo.toml index fcec53af3..0661ae692 100644 --- a/concurrency/Cargo.toml +++ b/concurrency/Cargo.toml @@ -6,7 +6,9 @@ publish.workspace = true version.workspace = true [features] +default = ["parking_lot"] loom = ["dep:loom", "concurrency-macros/loom"] +parking_lot = ["dep:parking_lot"] shuttle = ["dep:shuttle", "concurrency-macros/shuttle"] silence_clippy = ["concurrency-macros/silence_clippy"] # Do not manually enable this feature, let --all-features do it for you # Force the Mutex-based slot fallback even without a model checker. @@ -20,6 +22,7 @@ _strict_provenance = [] arc-swap = { workspace = true } concurrency-macros = { workspace = true, features = [] } loom = { workspace = true, optional = true, features = [] } +parking_lot = { workspace = true, optional = true, features = [] } shuttle = { workspace = true, optional = true, features = [] } static_assertions = { workspace = true } diff --git a/concurrency/src/sync/mod.rs b/concurrency/src/sync/mod.rs index 947f5d49b..f3234a237 100644 --- a/concurrency/src/sync/mod.rs +++ b/concurrency/src/sync/mod.rs @@ -4,21 +4,39 @@ //! Backend-routed synchronization primitives. //! //! Exposes a `parking_lot`-shaped surface for `Mutex` / `RwLock` that -//! compiles unchanged across backends. The default (non-model-checker) -//! backend currently routes through `std_backend` -- a thin -//! poison-as-panic wrapper around `std::sync::{Mutex, RwLock}` that -//! exposes naked guards (no `LockResult` to `.unwrap()` at call -//! sites). This workspace treats a crashed thread as a crashed -//! process, so silently inheriting state from a poisoned lock is -//! wrong; surfacing it as a panic propagates the failure correctly. +//! compiles unchanged across backends. //! -//! Loom and shuttle still re-export their raw `LockResult`-based -//! primitives at this point in the stack; subsequent PRs add the same -//! poison-as-panic wrap for those backends. +//! Selection (in priority order): +//! +//! * `loom` / `shuttle` features: raw re-export of the model-checker's +//! `LockResult`-based primitives. Subsequent PRs wrap these too. +//! * `parking_lot` feature (default): zero-cost re-export of +//! `parking_lot`'s naked-guard locks; the production hot path. +//! * Otherwise: `std_backend` -- a thin poison-as-panic wrapper around +//! `std::sync`. Same surface as the `parking_lot` re-export, one +//! extra match on acquire. Lets `--no-default-features` builds +//! compile without depending on `parking_lot`. + +#[cfg(all( + not(any(feature = "loom", feature = "shuttle")), + feature = "parking_lot", +))] +mod parking_lot_backend; +#[cfg(all( + not(any(feature = "loom", feature = "shuttle")), + feature = "parking_lot", +))] +pub use parking_lot_backend::*; -#[cfg(not(any(feature = "loom", feature = "shuttle")))] +#[cfg(all( + not(any(feature = "loom", feature = "shuttle")), + not(feature = "parking_lot"), +))] mod std_backend; -#[cfg(not(any(feature = "loom", feature = "shuttle")))] +#[cfg(all( + not(any(feature = "loom", feature = "shuttle")), + not(feature = "parking_lot"), +))] pub use std_backend::*; #[cfg(all( diff --git a/concurrency/src/sync/parking_lot_backend.rs b/concurrency/src/sync/parking_lot_backend.rs new file mode 100644 index 000000000..81b8b1542 --- /dev/null +++ b/concurrency/src/sync/parking_lot_backend.rs @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Open Network Fabric Authors + +//! Default-production backend: `parking_lot` locks layered on top of +//! `std::sync`. +//! +//! `parking_lot::{Mutex, RwLock}` already match the surface the rest +//! of the crate presents -- naked guards, no poison, fast contention +//! path. This module is a pure re-export so production builds pay no +//! wrapping cost. Everything that `parking_lot` doesn't ship +//! (`Arc`, `Weak`, `atomic`, `mpsc`, `Condvar`, `Once`, ...) comes +//! straight from `std::sync` so ordering semantics match a normal +//! release build. + +pub use parking_lot::{ + Mutex, MutexGuard, RwLock, RwLockReadGuard, RwLockUpgradableReadGuard, RwLockWriteGuard, +}; + +pub use std::sync::{ + Arc, Barrier, BarrierWaitResult, Condvar, LockResult, Once, OnceLock, OnceState, PoisonError, + TryLockError, TryLockResult, WaitTimeoutResult, Weak, atomic, mpsc, +};