Skip to content

Commit b9aa439

Browse files
committed
mason: add MC_DRIVE_FAULT_COUNT self-disarm to drive-fault feature
The CC-leg peer caught that OnceLock arming makes the fault permanent for the process lifetime, which breaks the fence+recover drive arc: a recovery pass would also be corrupted, and the only other disarm is a restart (which injects a variable the arc must not contain). Change: add MC_DRIVE_FAULT_COUNT=N (default 1 when unset or unparseable, read once alongside the existing MC_DRIVE_FAULT OnceLock). The fault fires for the FIRST N transform responses in the process, then self-disarms permanently. Implementation: - parse_drive_fault_count(): pure function, defaults to 1 - DRIVE_FAULT_REMAINING: AtomicUsize initialized alongside the arm selection - respond_transform: uses fetch_update with checked_sub so concurrent responses cannot underflow or double-fire past N. Total WARN lines in logs will equal exactly N. Tests (all under #[cfg(feature = "drive-fault")], pure-function style): - count=1 fires once then clean - count=3 fires exactly 3 then clean - default (no count) behaves as count=1 - unparseable count falls back to 1 - concurrent claim safety: loop of 7 claims with N=5 yields exactly 5 successes and 2 clean skips
1 parent 05df0b4 commit b9aa439

1 file changed

Lines changed: 176 additions & 3 deletions

File tree

crates/mc-module/src/lib.rs

Lines changed: 176 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9150,6 +9150,12 @@ fn replay_dream_task_response(response_json: &str) -> HandlerOutcome {
91509150
// scoped to transform responses only: respond_transform is the sole transform-response
91519151
// serializer, and facade tools and status/wrapup ops never route through it.
91529152
//
9153+
// Additionally, the fault self-disarms after MC_DRIVE_FAULT_COUNT firings (default 1)
9154+
// via a fetch_sub claim on DRIVE_FAULT_REMAINING. This prevents the fault from
9155+
// corrupting a recovery pass in the fence+recover drive arc — the only other disarm
9156+
// would be a restart, which injects a variable the arc must not contain. Total WARN
9157+
// lines in logs will equal exactly N, so miscounts are visible.
9158+
//
91539159
// Maintenance rule: never add another fault arm without the same absence-proof note in
91549160
// Cargo.toml and the fault-shape tests that pin the existing arms.
91559161
#[cfg(feature = "drive-fault")]
@@ -9172,13 +9178,46 @@ fn parse_drive_fault(raw: Option<&str>) -> Option<DriveFault> {
91729178
}
91739179
}
91749180

9181+
/// Parse MC_DRIVE_FAULT_COUNT: how many transform responses may fire the fault before
9182+
/// the mechanism self-disarms. Pure (no env access) so unit-testable without touching
9183+
/// process-global env or the process-wide OnceLock/AtomicUsize below.
9184+
///
9185+
/// Defaults to 1 when unset, empty, or unparseable. A value of 0 is treated as
9186+
/// unset (defaults to 1) since 0 would mean "never fire" which is indistinguishable
9187+
/// from not setting MC_DRIVE_FAULT at all.
9188+
#[cfg(feature = "drive-fault")]
9189+
fn parse_drive_fault_count(raw: Option<&str>) -> usize {
9190+
match raw.and_then(|s| s.parse::<usize>().ok()) {
9191+
Some(n) if n > 0 => n,
9192+
_ => 1,
9193+
}
9194+
}
9195+
9196+
/// Remaining fault-firings before the mechanism self-disarms permanently.
9197+
/// Initialized alongside the arm selection in `drive_fault()`.
9198+
#[cfg(feature = "drive-fault")]
9199+
static DRIVE_FAULT_REMAINING: std::sync::atomic::AtomicUsize =
9200+
std::sync::atomic::AtomicUsize::new(0);
9201+
91759202
/// The active fault arm, read from MC_DRIVE_FAULT once per process (first call wins via
9176-
/// OnceLock — no per-request getenv on the transform hot path).
9203+
/// OnceLock — no per-request getenv on the transform hot path). Also initializes
9204+
/// DRIVE_FAULT_REMAINING from MC_DRIVE_FAULT_COUNT so the count is set before any
9205+
/// respond_transform call can read it.
91779206
#[cfg(feature = "drive-fault")]
91789207
fn drive_fault() -> Option<DriveFault> {
91799208
use std::sync::OnceLock;
91809209
static FAULT: OnceLock<Option<DriveFault>> = OnceLock::new();
9181-
*FAULT.get_or_init(|| parse_drive_fault(std::env::var("MC_DRIVE_FAULT").ok().as_deref()))
9210+
*FAULT.get_or_init(|| {
9211+
let fault = parse_drive_fault(std::env::var("MC_DRIVE_FAULT").ok().as_deref());
9212+
// Initialize the remaining fault count alongside the arm selection.
9213+
// This runs exactly once per process (OnceLock), so DRIVE_FAULT_REMAINING
9214+
// is set before any respond_transform call can read it.
9215+
let count = parse_drive_fault_count(
9216+
std::env::var("MC_DRIVE_FAULT_COUNT").ok().as_deref(),
9217+
);
9218+
DRIVE_FAULT_REMAINING.store(count, std::sync::atomic::Ordering::Relaxed);
9219+
fault
9220+
})
91829221
}
91839222

91849223
/// Corrupt a transform response per the selected fault arm and log one loud WARN per
@@ -9218,9 +9257,25 @@ fn respond_transform(
92189257
// drive-fault: corrupt the response before it is serialized (see the SAFETY note
92199258
// above the fault helpers). No-op unless the feature is compiled in AND MC_DRIVE_FAULT
92209259
// selects an arm; must run before ck_messages is taken for the streaming placeholder.
9260+
//
9261+
// The fault fires at most N times (MC_DRIVE_FAULT_COUNT, default 1) then self-disarms
9262+
// permanently via a fetch_update claim on DRIVE_FAULT_REMAINING. This is critical for the
9263+
// fence+recover drive arc: a recovery pass must NOT be corrupted, and the only other
9264+
// disarm is a restart (which injects a variable the arc must not contain). The claim
9265+
// uses checked_sub so concurrent responses cannot underflow or double-fire past N —
9266+
// total WARN lines in logs will equal exactly N.
92219267
#[cfg(feature = "drive-fault")]
92229268
if let Some(fault) = drive_fault() {
9223-
apply_drive_fault(&mut response, fault);
9269+
// Claim one firing: fetch_update atomically decrements only if the count is > 0.
9270+
// If the count was already 0, checked_sub returns None and we skip — no underflow.
9271+
// If the count was > 0, the previous value is returned as Ok(prev) and we fire.
9272+
use std::sync::atomic::Ordering;
9273+
match DRIVE_FAULT_REMAINING.fetch_update(Ordering::AcqRel, Ordering::Acquire, |n| {
9274+
n.checked_sub(1)
9275+
}) {
9276+
Ok(prev) if prev > 0 => apply_drive_fault(&mut response, fault),
9277+
_ => {} // exhausted — response passes through cleanly
9278+
}
92249279
}
92259280
let response_encode_started_at = Instant::now();
92269281
let pass_timings = response.timings.clone();
@@ -12813,6 +12868,124 @@ mod tests {
1281312868
assert_eq!(value["status"], json!("ok"));
1281412869
}
1281512870

12871+
#[test]
12872+
#[cfg(feature = "drive-fault")]
12873+
fn drive_fault_count_parse_defaults_to_one() {
12874+
// Unset, empty, unparseable, and zero all default to 1.
12875+
assert_eq!(parse_drive_fault_count(None), 1);
12876+
assert_eq!(parse_drive_fault_count(Some("")), 1);
12877+
assert_eq!(parse_drive_fault_count(Some("not_a_number")), 1);
12878+
assert_eq!(parse_drive_fault_count(Some("0")), 1);
12879+
}
12880+
12881+
#[test]
12882+
#[cfg(feature = "drive-fault")]
12883+
fn drive_fault_count_parse_accepts_positive_values() {
12884+
assert_eq!(parse_drive_fault_count(Some("1")), 1);
12885+
assert_eq!(parse_drive_fault_count(Some("3")), 3);
12886+
assert_eq!(parse_drive_fault_count(Some("100")), 100);
12887+
}
12888+
12889+
#[test]
12890+
#[cfg(feature = "drive-fault")]
12891+
fn drive_fault_count_one_fires_once_then_clean() {
12892+
// Simulate count=1: claim one firing, then verify subsequent claims are no-ops.
12893+
// We test the claim logic directly by manipulating DRIVE_FAULT_REMAINING and
12894+
// checking the fetch_update guard condition.
12895+
DRIVE_FAULT_REMAINING.store(1, std::sync::atomic::Ordering::Relaxed);
12896+
12897+
// First claim: should fire (Ok(1) with prev=1 > 0).
12898+
let result = DRIVE_FAULT_REMAINING.fetch_update(
12899+
std::sync::atomic::Ordering::AcqRel,
12900+
std::sync::atomic::Ordering::Acquire,
12901+
|n| n.checked_sub(1),
12902+
);
12903+
assert_eq!(result, Ok(1), "first claim should fire (prev=1)");
12904+
assert_eq!(
12905+
DRIVE_FAULT_REMAINING.load(std::sync::atomic::Ordering::Relaxed),
12906+
0,
12907+
"count exhausted after one claim"
12908+
);
12909+
12910+
// Second claim: should NOT fire (Err(0) — checked_sub returns None).
12911+
let result = DRIVE_FAULT_REMAINING.fetch_update(
12912+
std::sync::atomic::Ordering::AcqRel,
12913+
std::sync::atomic::Ordering::Acquire,
12914+
|n| n.checked_sub(1),
12915+
);
12916+
assert_eq!(result, Err(0), "second claim should be clean (no firing)");
12917+
// Must not underflow: stays at 0.
12918+
assert_eq!(
12919+
DRIVE_FAULT_REMAINING.load(std::sync::atomic::Ordering::Relaxed),
12920+
0,
12921+
"count must not underflow past 0"
12922+
);
12923+
}
12924+
12925+
#[test]
12926+
#[cfg(feature = "drive-fault")]
12927+
fn drive_fault_count_three_fires_exactly_three_then_clean() {
12928+
DRIVE_FAULT_REMAINING.store(3, std::sync::atomic::Ordering::Relaxed);
12929+
12930+
for i in 1..=3 {
12931+
let result = DRIVE_FAULT_REMAINING.fetch_update(
12932+
std::sync::atomic::Ordering::AcqRel,
12933+
std::sync::atomic::Ordering::Acquire,
12934+
|n| n.checked_sub(1),
12935+
);
12936+
// fetch_update returns Ok(prev) where prev is the value BEFORE the update.
12937+
// With count=3, calls return Ok(3), Ok(2), Ok(1) — all > 0.
12938+
assert!(
12939+
result.is_ok() && result.unwrap() > 0,
12940+
"claim {i} should fire (result={result:?})"
12941+
);
12942+
}
12943+
12944+
// Fourth claim: exhausted.
12945+
let result = DRIVE_FAULT_REMAINING.fetch_update(
12946+
std::sync::atomic::Ordering::AcqRel,
12947+
std::sync::atomic::Ordering::Acquire,
12948+
|n| n.checked_sub(1),
12949+
);
12950+
assert_eq!(result, Err(0), "fourth claim should be clean");
12951+
assert_eq!(
12952+
DRIVE_FAULT_REMAINING.load(std::sync::atomic::Ordering::Relaxed),
12953+
0,
12954+
"count must not underflow past 0"
12955+
);
12956+
}
12957+
12958+
#[test]
12959+
#[cfg(feature = "drive-fault")]
12960+
fn drive_fault_count_concurrent_claim_safety() {
12961+
// Simulate N concurrent claims by iterating a loop that mimics the atomic claim
12962+
// pattern. With N=5 and 7 claims, exactly 5 should succeed (Ok(prev) with prev > 0)
12963+
// and 2 should be clean (Err(0)). This validates the checked_sub semantics without
12964+
// requiring real threads.
12965+
DRIVE_FAULT_REMAINING.store(5, std::sync::atomic::Ordering::Relaxed);
12966+
12967+
let mut fired = 0usize;
12968+
let mut clean = 0usize;
12969+
for _ in 0..7 {
12970+
match DRIVE_FAULT_REMAINING.fetch_update(
12971+
std::sync::atomic::Ordering::AcqRel,
12972+
std::sync::atomic::Ordering::Acquire,
12973+
|n| n.checked_sub(1),
12974+
) {
12975+
Ok(prev) if prev > 0 => fired += 1,
12976+
_ => clean += 1,
12977+
}
12978+
}
12979+
12980+
assert_eq!(fired, 5, "exactly 5 claims should fire");
12981+
assert_eq!(clean, 2, "remaining 2 claims should be clean");
12982+
assert_eq!(
12983+
DRIVE_FAULT_REMAINING.load(std::sync::atomic::Ordering::Relaxed),
12984+
0,
12985+
"count must not underflow past 0"
12986+
);
12987+
}
12988+
1281612989
#[tokio::test(flavor = "current_thread")]
1281712990
async fn serve_native_false_is_response_byte_identical_for_all_profiles() {
1281812991
let producer = Arc::new(ProducerState::default());

0 commit comments

Comments
 (0)