Race borrowed pool-slot permits without per-slot Arc cloning (#646)

.github/workflows/ci.yml

@@ -95,7 +95,13 @@ jobs:
       - name: Install Whitaker
         uses: leynos/shared-actions/.github/actions/install-whitaker@8e6a325765ba382691d1b1c65fcccf1fc1d1b00c
       - name: Lint
-        run: make lint
+        # Dylint 6.0.1 expects a local master branch in its rust-clippy clone.
+        # Limit the compatibility wrapper to this step; all other Git calls
+        # pass through to the runner's normal Git executable.
+        run: |
+          export DYLINT_REAL_GIT="$(command -v git)"
+          export PATH="$GITHUB_WORKSPACE/scripts/dylint-git:$PATH"
+          make lint
       - name: Setup uv
         uses: astral-sh/setup-uv@18d369b23da25a81e9458561248e1aafff6a70fa
         with:
@@ -134,6 +140,8 @@ jobs:
       # about whether the gate ran.
       - name: Feature-gate compile checks
         run: make test-feature-gate
+      - name: Pool without metrics compile check
+        run: make test-pool-no-metrics
       # Coverage runs on pull requests only and drives the ratchet gate.
       # Main-branch coverage — the CodeScene upload plus the authoritative
       # ratchet baseline every pull request compares against — is produced

Makefile

@@ -1,4 +1,4 @@
-.PHONY: help all clean test test-doc test-ui test-feature-gate bless-ui test-workflow-contracts test-loom doctest-benchmark
+.PHONY: help all clean test test-doc test-ui test-feature-gate test-pool-no-metrics bless-ui test-workflow-contracts test-loom doctest-benchmark
 .PHONY: bench-codec bench-runtime-ownership bench build dev-build dev-test release lint fmt check-fmt markdownlint nixie typecheck
 .PHONY: spelling
 .PHONY: install-kani check-kani-version install-verus run-verus test-verification kani \
@@ -123,6 +123,9 @@ test-ui: ## Run trybuild compile-time API tests
 test-feature-gate: ## Verify feature-gated APIs are actually absent when disabled
 	./scripts/check-feature-gate.sh
 
+test-pool-no-metrics: ## Check the pool builds without metrics
+	RUSTFLAGS="$(DEV_WARNING_FLAGS)" $(CARGO) $(DEV_FAST) check --lib --no-default-features --features pool,serializer-bincode $(BUILD_JOBS)
+
 # Developer-only: review every changed snapshot; this target must not run in CI.
 bless-ui: ## Refresh trybuild snapshots for deliberate fixture changes
 	TRYBUILD=overwrite $(CARGO) test --test compile_error

benches/runtime_ownership_baselines/scenarios.rs

@@ -20,6 +20,7 @@ use wireframe_testing::runtime_ownership_benchmarks::{
     PushWorkload,
     StartupPath,
     StartupWorkload,
+    contended_fifo_pool_workloads,
 };
 
 /// Criterion sample count for every runtime-ownership group.
@@ -245,21 +246,17 @@ pub(crate) fn uncontended_pool_workloads() -> Vec<PoolWorkload> {
         .collect()
 }
 
-/// The contended pool workloads reported as bounded observations, not timings.
+/// Contended workloads reported as bounded timing observations.
 ///
-/// Both fairness policies run over one pool size: the issue asks for the
-/// policies to be compared, not for every policy to be repeated at every size.
+/// FIFO runs cover all baseline pool sizes. Round-robin remains the existing
+/// size-four comparison so its label and workload stay directly comparable.
 pub(crate) fn contended_pool_workloads() -> Vec<PoolWorkload> {
-    [
-        AcquisitionMode::ContendedFifo,
-        AcquisitionMode::ContendedRoundRobin,
-    ]
-    .into_iter()
-    .map(|mode| PoolWorkload {
+    let mut workloads = contended_fifo_pool_workloads().to_vec();
+    workloads.push(PoolWorkload {
         pool_size: CONTENDED_POOL_SIZE,
-        mode,
+        mode: AcquisitionMode::ContendedRoundRobin,
         contenders: CONTENDED_POOL_CONTENDERS,
         ..PoolWorkload::default()
-    })
-    .collect()
+    });
+    workloads
 }

benches/runtime_ownership_baselines_alloc/main.rs

@@ -39,14 +39,14 @@
 //!
 //! # Contended pool acquisition
 //!
-//! Contended pool acquisition is counted once, after `Criterion::final_summary`,
-//! and reported as a `tracing` event rather than timed. Sustained contention
-//! intermittently fails or stalls the pool acquire path — a pre-existing defect
-//! recorded in `docs/runtime-ownership-baselines.md` and belonging to ADR 013 /
-//! issue #647. Criterion's timing pass runs far more iterations than an
-//! allocation window, so it reaches that interleaving almost every run; that is
-//! how this binary failed on its first execution. The count itself is unaffected
-//! because it comes from a single counted window.
+//! FIFO contended pool acquisition is counted once per baseline pool size, after
+//! `Criterion::final_summary`, and reported as a `tracing` event rather than
+//! timed. Sustained contention intermittently fails or stalls the pool acquire
+//! path — a pre-existing defect recorded in `docs/runtime-ownership-baselines.md`
+//! and belonging to ADR 013 / issue #647. Criterion's timing pass runs far more
+//! iterations than an allocation window, so it reaches that interleaving almost
+//! every run; that is how this binary failed on its first execution. Each count
+//! comes from one bracketed window.
 //!
 //! # Layout
 //!

benches/runtime_ownership_baselines_alloc/pool_bracket.rs

@@ -13,10 +13,7 @@ use wireframe_testing::{
     runtime_ownership_benchmarks::{PoolHarness, PoolWorkload},
 };
 
-use crate::{
-    probes::expect_bench,
-    scopes::{ALLOCATION_ITERATIONS, AllocationWorkloads},
-};
+use crate::{probes::expect_bench, scopes::ALLOCATION_ITERATIONS};
 
 /// Run the pool scope the way the timing pass should replay it.
 ///
@@ -103,7 +100,7 @@ fn pool_bracket_readings(
 }
 
 /// Build the workload the bracket checks measure, at their own parameters.
-fn bracket_workload() -> PoolWorkload { AllocationWorkloads::new().pool }
+fn bracket_workload() -> PoolWorkload { PoolWorkload::default() }
 
 /// Prove the pool scope's bracketed window really excludes pool construction.
 ///

benches/runtime_ownership_baselines_alloc/reporting.rs

@@ -2,15 +2,14 @@
 //!
 //! A count is recorded before its timing pass and emitted under a label whose
 //! identity does not depend on it, so Criterion's saved results stay comparable
-//! between runs. The contended pool scope is published as an event alone: it
-//! cannot survive a Criterion timing pass, but the issue still asks for its
-//! number.
+//! between runs. FIFO contended pool counts are published as events alone: they
+//! cannot survive a Criterion timing pass, but the baseline needs their counts.
 
 use std::{hint::black_box, time::Duration};
 
 use criterion::{BenchmarkId, Criterion};
 use tokio::runtime::Runtime;
-use wireframe_testing::runtime_ownership_benchmarks::{AcquisitionMode, PoolWorkload};
+use wireframe_testing::runtime_ownership_benchmarks::contended_fifo_pool_workloads;
 
 use crate::{
     pool_bracket::count_pool_acquisition_window,
@@ -58,26 +57,6 @@ where
     total
 }
 
-/// Contended pool workload, recorded as an observation rather than a timing.
-///
-/// Contenders must outnumber slots for the pool to be exhausted, and the
-/// default `contenders` of 16 clears the default pool size of 4 comfortably.
-///
-/// This workload is deliberately absent from `allocation_scopes`: sustained
-/// contention intermittently stalls the pool acquire path or fails it with a
-/// spurious `disconnected`, and Criterion's timing pass runs far more
-/// iterations than the four in an allocation window, so it reaches that
-/// interleaving almost every run. Measuring it here once, under a counted
-/// window, records the number the issue asks for without handing the scenario
-/// to a pass that cannot survive it. See `docs/runtime-ownership-baselines.md`.
-fn contended_pool_workload() -> PoolWorkload {
-    PoolWorkload {
-        pool_size: 4,
-        mode: AcquisitionMode::ContendedFifo,
-        ..PoolWorkload::default()
-    }
-}
-
 /// Emit a successful contended allocation count as an event.
 ///
 /// Kept as its own function because a `tracing` macro expands to an
@@ -111,19 +90,19 @@ fn report_contended_allocations(label: &str, outcome: Result<usize, String>) {
     }
 }
 
-/// Record the contended pool allocation count as a bounded observation.
+/// Record FIFO contended allocation counts across the baseline pool sizes.
 ///
 /// Run once after Criterion has finished, so a failure is reported rather than
 /// aborting the benchmark run. The count comes from the same bracketed window the
 /// uncontended pool scope uses, so the two are comparable; only the repetition
 /// and the timing are dropped, because repetition is what the acquire-path defect
 /// makes unreliable. See `docs/runtime-ownership-baselines.md`.
 pub(crate) fn observe_contended_pool_allocations(runtime: &Runtime) {
-    let workload = contended_pool_workload();
-    let label = workload.label();
-    let outcome = count_pool_acquisition_window(runtime, workload, ALLOCATION_ITERATIONS);
-
-    report_contended_allocations(&label, outcome);
+    for workload in contended_fifo_pool_workloads() {
+        let label = workload.label();
+        let outcome = count_pool_acquisition_window(runtime, workload, ALLOCATION_ITERATIONS);
+        report_contended_allocations(&label, outcome);
+    }
 }
 
 /// Install a subscriber for the allocation binary's own events only.

benches/runtime_ownership_baselines_alloc/scopes.rs

@@ -17,6 +17,7 @@ use wireframe_testing::{
         AcquisitionMode,
         ActorWorkload,
         CancellationMode,
+        POOL_SIZES,
         PoolWorkload,
         PushScenario,
         PushWorkload,
@@ -128,7 +129,7 @@ impl<'a> AllocationScope<'a> {
 /// measurement's own counters are dropped: a scope closure's only job is to
 /// perform the work the counting window observes. Adapting every scope through
 /// one helper is what keeps [`allocation_scopes`] short enough to read and keeps
-/// the six scopes visibly identical in shape.
+/// the scopes visibly identical in shape.
 fn run_measurement<T, E: std::fmt::Display>(
     runtime: &Runtime,
     measurement: impl std::future::Future<Output = Result<T, E>>,
@@ -141,11 +142,10 @@ fn run_measurement<T, E: std::fmt::Display>(
 
 /// The workload parameters this binary records allocations for, by scope.
 ///
-/// Split from [`allocation_scopes`] because the six literal constructions alone
-/// push that function past the workspace's line limit. Keeping the parameters in
-/// one block also makes them easy to compare at a glance, which is their point:
-/// each differs from the timing binary's defaults deliberately, to keep the
-/// allocation windows short enough to compare as small integers.
+/// Split from [`allocation_scopes`] so its closures stay below the workspace's
+/// function-size limit. Keeping the parameters in one block also makes them
+/// easy to compare at a glance, which is their point: each differs from the
+/// timing binary's defaults deliberately, to keep allocation windows short.
 pub(crate) struct AllocationWorkloads {
     /// Actor drain, with cancellation disabled so the window covers the loop.
     actor: ActorWorkload,
@@ -157,12 +157,12 @@ pub(crate) struct AllocationWorkloads {
     startup_prepared: StartupWorkload,
     /// The per-connection factory path, which prepares once per connection.
     startup_per_connection: StartupWorkload,
-    /// Uncontended acquire/release, the only pool mode safe to time.
-    pub(crate) pool: PoolWorkload,
+    /// Uncontended acquire/release at the named baseline pool sizes.
+    pool_workloads: [PoolWorkload; POOL_SIZES.len()],
 }
 
 impl AllocationWorkloads {
-    /// Build the six scopes at their allocation-window parameters.
+    /// Build the scopes at their allocation-window parameters.
     pub(crate) fn new() -> Self {
         Self {
             actor: ActorWorkload {
@@ -191,11 +191,11 @@ impl AllocationWorkloads {
                 path: StartupPath::PerConnectionFactory,
                 ..StartupWorkload::default()
             },
-            pool: PoolWorkload {
-                pool_size: 4,
+            pool_workloads: POOL_SIZES.map(|pool_size| PoolWorkload {
+                pool_size,
                 mode: AcquisitionMode::Uncontended,
                 ..PoolWorkload::default()
-            },
+            }),
         }
     }
 }
@@ -209,7 +209,7 @@ pub(crate) fn allocation_scopes(runtime: &Runtime) -> Vec<AllocationScope<'_>> {
     let workloads = AllocationWorkloads::new();
     let iterations = ALLOCATION_ITERATIONS;
 
-    vec![
+    let mut scopes = vec![
         AllocationScope::windowed(workloads.actor.label(), move || {
             run_measurement(runtime, measure_actor_drain(workloads.actor, iterations))
         }),
@@ -231,12 +231,18 @@ pub(crate) fn allocation_scopes(runtime: &Runtime) -> Vec<AllocationScope<'_>> {
                 measure_startup(workloads.startup_per_connection, iterations),
             )
         }),
-        AllocationScope {
-            label: workloads.pool.label(),
-            count: Box::new(move || {
-                count_pool_acquisition_window(runtime, workloads.pool, iterations)
+    ];
+    scopes.extend(
+        workloads
+            .pool_workloads
+            .into_iter()
+            .map(|workload| AllocationScope {
+                label: workload.label(),
+                count: Box::new(move || {
+                    count_pool_acquisition_window(runtime, workload, iterations)
+                }),
+                replay: Box::new(move || run_pool_acquisition(runtime, workload, iterations)),
             }),
-            replay: Box::new(move || run_pool_acquisition(runtime, workloads.pool, iterations)),
-        },
-    ]
+    );
+    scopes
 }

docs/adr-013-client-pool-scheduler-and-slot-ownership.md

@@ -541,12 +541,44 @@ are internal scaffolding removed before this ADR flips to Accepted.
 
 ### Phase 4: use scoped permit races
 
-Remove the per-slot `Arc<PoolCore>` clones and `'static` boxed slot futures
-from the current permit race
-([#646](https://github.com/leynos/wireframe/issues/646)). The current code
-clones the core once for each slot future. The scoped-race rework deliberately
-follows the actor phase so the races are written once against the actor's grant
-loop rather than twice.
+Use the fixed slot storage and index-based leases introduced by #645 to remove
+the per-slot `Arc<PoolCore>` clones and `'static` boxed futures from permit
+acquisition ([#646](https://github.com/leynos/wireframe/issues/646)). Advance
+the round-robin cursor once, probe the rotated slots without allocation, then
+use `FuturesUnordered` only when every slot is blocked. Each blocked future
+borrows its stable `PoolSlot` and registers a FIFO semaphore waiter when
+polled. Dropping the stream cancels losing acquisitions and releases any
+permits they acquired before the winner was selected.
+
+The immediate probe preserves deterministic start-slot behaviour because
+`FuturesUnordered` does not promise poll order. It allocates no storage
+proportional to pool size when a probe succeeds. A fully blocked race allocates
+one `FuturesUnordered` task node per slot. The serial service worker permits at
+most one active race per pool scheduler, and the scheduler keeps its existing
+fairness policy.
+
+The service worker retains its `Arc<PoolCore>` across `tokio::select!`, so the
+acquisition can borrow the core while shutdown and closed-sender branches
+remain active. This avoids a per-race or per-slot core clone.
+
+The race reuses the `rotation_start` and `rotation_indices` helpers from
+[#536](https://github.com/leynos/wireframe/issues/536). The iterator exposes
+`Clone` so the probe and blocked race share one rotated order after exactly one
+cursor advance, without materializing an index collection. The bounded
+acquisition metric wrappers introduced alongside those helpers remain around
+both immediate and asynchronous selection.
+
+The scheduler and metrics entry points use their respective `mod.rs` files,
+following the module layout established by #798. The metrics-disabled pool
+build uses the existing `tokio-util` `io` feature. Validation follows the
+repository-pinned Rust toolchain and CI action versions; the historical
+baseline captures retain the toolchain metadata from their runs.
+
+Blocked races record their starts, terminal outcomes, and durations. A guard
+records cancellation when the scheduler drops an unfinished acquisition; the
+immediate probe does not create a blocked-race observation. The guard borrows a
+monotonic clock: production uses `Instant`, and recorder-backed tests inject a
+deterministic clock to verify the duration value.
 
 ### Phase 5: integrate existing safety work
 
@@ -575,11 +607,18 @@ Complete or coordinate the following, plus any remaining useful scope from
   rather than passing.
 - Dropping all public handles without `close` eventually releases the pool
   core, including with the `JoinHandle` held by the non-shared close owner.
-- Loom or deterministic state-machine tests cover enqueue, capacity,
-  cancellation, deregistration, and shutdown races, naming at minimum: the
-  pending-flag RAII clear when a permit race is dropped mid-`select!`, and a
-  permit won exactly as its waiter's receiver drops. No loom coverage exists
-  for the pool today; the only loom suite exercises the push queues.
+- Deterministic current-thread tests manually poll pool permit races to cover
+  registration, simultaneous readiness, winner and loser permit ownership,
+  outer cancellation, and close. A saturated-core test verifies shutdown
+  rejects an acquisition before it queues behind slot waiters. This uses
+  Tokio's actual semaphore and scheduler primitives, whose queue registration
+  and drop behaviour a downstream Loom build cannot schedule. The Loom suites
+  cover push queues and connection-gauge transitions; they do not model the
+  client pool.
+- Bounded tests enumerate every non-empty readiness set and release order for
+  pools of two, three, and four slots. They check winner and loser capacity,
+  cancellation after release, shutdown after registration, and cancellation
+  after shutdown, using Tokio's actual semaphores.
 - A panic injected into the scheduler task surfaces as `SchedulerFailed`
   with the required log event and metric, not as a silent hang or a clean
   `PoolClosed`.