Skip to content

Commit 8bd5fb4

Browse files
test: isolate missing-env reporting state (Fixes #500) (#501)
## Summary - inject missing-env reporting atomics into the state-machine helpers - keep production wrappers on the process-global state - give each unit test an independent local atomic instead of a shared mutex/global - prevent configure tests from racing or poisoning missing-env tests ## Validation - 20 consecutive `cargo test -p pet --bin pet --quiet` runs - `.\scripts\rust-precommit.ps1` Fixes #500 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent cc8f859 commit 8bd5fb4

1 file changed

Lines changed: 74 additions & 40 deletions

File tree

‎crates/pet/src/jsonrpc.rs‎

Lines changed: 74 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -941,17 +941,29 @@ fn is_current_generation(
941941
fn try_begin_missing_env_reporting(
942942
configuration: &RwLock<ConfigurationState>,
943943
refresh_generation: u64,
944+
) -> bool {
945+
try_begin_missing_env_reporting_with_state(
946+
&MISSING_ENVS_REPORTING_STATE,
947+
configuration,
948+
refresh_generation,
949+
)
950+
}
951+
952+
fn try_begin_missing_env_reporting_with_state(
953+
reporting_state: &AtomicU64,
954+
configuration: &RwLock<ConfigurationState>,
955+
refresh_generation: u64,
944956
) -> bool {
945957
loop {
946-
let current_state = MISSING_ENVS_REPORTING_STATE.load(Ordering::Acquire);
958+
let current_state = reporting_state.load(Ordering::Acquire);
947959
if current_state == MISSING_ENVS_COMPLETED {
948960
return false;
949961
}
950962
if current_state != MISSING_ENVS_AVAILABLE && current_state >= refresh_generation {
951963
return false;
952964
}
953965

954-
if MISSING_ENVS_REPORTING_STATE
966+
if reporting_state
955967
.compare_exchange(
956968
current_state,
957969
refresh_generation,
@@ -964,7 +976,11 @@ fn try_begin_missing_env_reporting(
964976
return true;
965977
}
966978

967-
release_missing_env_reporting_if_stale(configuration, refresh_generation);
979+
release_missing_env_reporting_if_stale_with_state(
980+
reporting_state,
981+
configuration,
982+
refresh_generation,
983+
);
968984
return false;
969985
}
970986
}
@@ -973,9 +989,21 @@ fn try_begin_missing_env_reporting(
973989
fn release_missing_env_reporting_if_stale(
974990
configuration: &RwLock<ConfigurationState>,
975991
refresh_generation: u64,
992+
) {
993+
release_missing_env_reporting_if_stale_with_state(
994+
&MISSING_ENVS_REPORTING_STATE,
995+
configuration,
996+
refresh_generation,
997+
);
998+
}
999+
1000+
fn release_missing_env_reporting_if_stale_with_state(
1001+
reporting_state: &AtomicU64,
1002+
configuration: &RwLock<ConfigurationState>,
1003+
refresh_generation: u64,
9761004
) {
9771005
if !is_current_generation(configuration, refresh_generation) {
978-
let _ = MISSING_ENVS_REPORTING_STATE.compare_exchange(
1006+
let _ = reporting_state.compare_exchange(
9791007
refresh_generation,
9801008
MISSING_ENVS_AVAILABLE,
9811009
Ordering::AcqRel,
@@ -985,14 +1013,17 @@ fn release_missing_env_reporting_if_stale(
9851013
}
9861014

9871015
fn complete_missing_env_reporting(refresh_generation: u64) {
988-
let _ = MISSING_ENVS_REPORTING_STATE.compare_exchange(
1016+
complete_missing_env_reporting_with_state(&MISSING_ENVS_REPORTING_STATE, refresh_generation);
1017+
}
1018+
1019+
fn complete_missing_env_reporting_with_state(reporting_state: &AtomicU64, refresh_generation: u64) {
1020+
let _ = reporting_state.compare_exchange(
9891021
refresh_generation,
9901022
MISSING_ENVS_COMPLETED,
9911023
Ordering::AcqRel,
9921024
Ordering::Acquire,
9931025
);
9941026
}
995-
9961027
fn execute_refresh(
9971028
context: &Context,
9981029
refresh_options: &RefreshOptions,
@@ -1485,8 +1516,6 @@ mod tests {
14851516
telemetry: Mutex<Vec<TelemetryEvent>>,
14861517
}
14871518

1488-
static MISSING_ENVS_TEST_LOCK: Mutex<()> = Mutex::new(());
1489-
14901519
struct LockCheckingReporter {
14911520
configuration: Arc<RwLock<ConfigurationState>>,
14921521
reported: Mutex<bool>,
@@ -1928,50 +1957,54 @@ mod tests {
19281957

19291958
#[test]
19301959
fn test_stale_generation_does_not_begin_missing_env_reporting() {
1931-
let _guard = MISSING_ENVS_TEST_LOCK.lock().unwrap();
1932-
MISSING_ENVS_REPORTING_STATE.store(MISSING_ENVS_AVAILABLE, Ordering::Release);
1960+
let reporting_state = AtomicU64::new(MISSING_ENVS_AVAILABLE);
19331961
let configuration = RwLock::new(ConfigurationState {
19341962
generation: 2,
19351963
config: Configuration::default(),
19361964
});
19371965

1938-
assert!(!try_begin_missing_env_reporting(&configuration, 1));
1966+
assert!(!try_begin_missing_env_reporting_with_state(
1967+
&reporting_state,
1968+
&configuration,
1969+
1,
1970+
));
19391971
assert_eq!(
1940-
MISSING_ENVS_REPORTING_STATE.load(Ordering::Acquire),
1972+
reporting_state.load(Ordering::Acquire),
19411973
MISSING_ENVS_AVAILABLE
19421974
);
19431975
}
19441976

19451977
#[test]
19461978
fn test_stale_generation_releases_missing_env_reporting_slot() {
1947-
let _guard = MISSING_ENVS_TEST_LOCK.lock().unwrap();
1948-
MISSING_ENVS_REPORTING_STATE.store(2, Ordering::Release);
1979+
let reporting_state = AtomicU64::new(2);
19491980
let configuration = RwLock::new(ConfigurationState {
19501981
generation: 3,
19511982
config: Configuration::default(),
19521983
});
19531984

1954-
release_missing_env_reporting_if_stale(&configuration, 2);
1985+
release_missing_env_reporting_if_stale_with_state(&reporting_state, &configuration, 2);
19551986

19561987
assert_eq!(
1957-
MISSING_ENVS_REPORTING_STATE.load(Ordering::Acquire),
1988+
reporting_state.load(Ordering::Acquire),
19581989
MISSING_ENVS_AVAILABLE
19591990
);
19601991
}
19611992

19621993
#[test]
19631994
fn test_newer_generation_can_claim_missing_env_reporting_after_older_reservation() {
1964-
let _guard = MISSING_ENVS_TEST_LOCK.lock().unwrap();
1965-
MISSING_ENVS_REPORTING_STATE.store(1, Ordering::Release);
1995+
let reporting_state = AtomicU64::new(1);
19661996
let configuration = RwLock::new(ConfigurationState {
19671997
generation: 2,
19681998
config: Configuration::default(),
19691999
});
19702000

1971-
assert!(try_begin_missing_env_reporting(&configuration, 2));
1972-
assert_eq!(MISSING_ENVS_REPORTING_STATE.load(Ordering::Acquire), 2);
2001+
assert!(try_begin_missing_env_reporting_with_state(
2002+
&reporting_state,
2003+
&configuration,
2004+
2,
2005+
));
2006+
assert_eq!(reporting_state.load(Ordering::Acquire), 2);
19732007
}
1974-
19752008
#[test]
19762009
fn test_refresh_coordinator_joins_identical_requests() {
19772010
let coordinator = RefreshCoordinator::default();
@@ -2674,39 +2707,40 @@ mod tests {
26742707
));
26752708
}
26762709

2677-
/// Test for #395: configure resets MISSING_ENVS_REPORTING_STATE so that
2678-
/// subsequent refreshes can trigger missing-env reporting again.
2710+
/// Test for #395: configure resets missing-env state so that subsequent
2711+
/// refreshes can trigger reporting again.
26792712
#[test]
26802713
fn test_configure_resets_completed_missing_env_reporting() {
2681-
let _guard = MISSING_ENVS_TEST_LOCK.lock().unwrap();
2682-
2714+
let reporting_state = AtomicU64::new(MISSING_ENVS_AVAILABLE);
26832715
let configuration = Arc::new(RwLock::new(ConfigurationState {
26842716
generation: 1,
26852717
config: Configuration::default(),
26862718
}));
26872719

2688-
// Simulate a completed first refresh.
2689-
MISSING_ENVS_REPORTING_STATE.store(MISSING_ENVS_AVAILABLE, Ordering::Release);
2690-
assert!(try_begin_missing_env_reporting(configuration.as_ref(), 1));
2691-
complete_missing_env_reporting(1);
2692-
2693-
// Missing-env reporting is now exhausted.
2694-
assert!(!try_begin_missing_env_reporting(configuration.as_ref(), 1));
2720+
assert!(try_begin_missing_env_reporting_with_state(
2721+
&reporting_state,
2722+
configuration.as_ref(),
2723+
1,
2724+
));
2725+
complete_missing_env_reporting_with_state(&reporting_state, 1);
2726+
assert!(!try_begin_missing_env_reporting_with_state(
2727+
&reporting_state,
2728+
configuration.as_ref(),
2729+
1,
2730+
));
26952731

2696-
// Simulate what handle_configure does: bump generation and reset.
26972732
{
26982733
let mut state = configuration.write().unwrap();
26992734
state.generation = 2;
2700-
MISSING_ENVS_REPORTING_STATE.store(MISSING_ENVS_AVAILABLE, Ordering::Release);
2735+
reporting_state.store(MISSING_ENVS_AVAILABLE, Ordering::Release);
27012736
}
27022737

2703-
// Missing-env reporting should work again for the new generation.
2704-
assert!(try_begin_missing_env_reporting(configuration.as_ref(), 2));
2705-
2706-
// Cleanup.
2707-
MISSING_ENVS_REPORTING_STATE.store(MISSING_ENVS_AVAILABLE, Ordering::Release);
2738+
assert!(try_begin_missing_env_reporting_with_state(
2739+
&reporting_state,
2740+
configuration.as_ref(),
2741+
2,
2742+
));
27082743
}
2709-
27102744
/// Test for #461: refresh-side `configuration.read()` callers must not
27112745
/// block on the configure thread while it iterates `locator.configure()`.
27122746
#[test]

0 commit comments

Comments
 (0)