Skip to content

test(control_plane): fix flaky env-var data race in unit tests - #318

Merged
zzylol merged 1 commit into
mainfrom
fix/control-plane-env-race
May 24, 2026
Merged

zzylol merged 1 commit into
mainfrom
fix/control-plane-env-race

Conversation

@zzylol

@zzylol zzylol commented May 24, 2026

Copy link
Copy Markdown
Contributor

Summary

control_plane unit tests mutate process-global environment via std::env::set_var / std::env::remove_var. cargo test runs #[test]/#[tokio::test] functions on parallel threads within a single process, so these mutations raced:

  • Three independent module-local mutexes (replan::tests::TYPED_ENV_GUARD, emit::stage_config::tests::MEMORY_LIMIT_ENV_LOCK) did not serialize against each other. Concurrent setenv/unsetenv (documented as unsound in a multithreaded program) raced on the one global environ table.
  • ~50 env-reading tests (notably the mvp46_* routing-shape assertions) held no lock, so they could transiently observe a concurrent writer's ASAP_EDGE_FUSED=1 and assert against the wrong (fused vs. routing) emit shape.

Result: nondeterministic failures depending on thread scheduling (reproduced reliably: 6 / 2 / 4 random mvp46_* failures across three full --lib runs before the fix).

The racy variables are ASAP_EDGE_FUSED, ASAP_AGENT_MEMORY_LIMIT_MIB, and USE_TYPED_STAGE_SPLIT (the originally-filed ASAP_EDGE_FUSED-only framing was incomplete).

Fix

  • Add a crate-wide #[cfg(test)] test_support module in lib.rs exposing one shared env_lock() mutex plus an RAII EnvVarGuard (set/unset on construct, restore the prior value on drop — panic-safe).
  • Route every lib unit test that reads or writes a process-global env var through that single lock, regardless of module (emit/stage_config.rs, emit/mod.rs, emit/trait_def.rs, replan.rs). The three old per-module locks are removed.
  • Production code is unchanged — this is purely a test-hygiene fix. main.rs keeps its own guard (it is a separate test binary and is already internally consistent).

No new dependency added (reuses the existing static Mutex pattern the crate already used in three places, now unified into one).

Test plan

  • cargo test -p control_plane --lib x5 — 769 passed; 0 failed every run (was flaky before).
  • cargo test -p control_plane (lib + bin) x3 — 769 + 28 passed, no failures.
  • cargo build -p control_plane clean.
  • cargo clippy -p control_plane --lib --tests — no new warnings from the changed files (pre-existing unused-import / vec_init_then_push lints in untouched production code remain).

🤖 Generated with Claude Code

…x flake

control_plane unit tests mutate process-global env (ASAP_EDGE_FUSED,
ASAP_AGENT_MEMORY_LIMIT_MIB, USE_TYPED_STAGE_SPLIT) via set_var/remove_var.
cargo test runs #[test] fns in parallel threads in a single process, so the
three previously-independent module-local mutexes did not serialize against
each other: concurrent setenv/unsetenv calls (documented as unsound) raced,
and env-reading tests (e.g. the mvp46_* routing assertions) transiently
observed a writer's ASAP_EDGE_FUSED=1, yielding nondeterministic failures.

Centralize a single crate-wide test_support::env_lock() mutex plus an RAII
EnvVarGuard (set/unset on construct, restore prior value on drop) in lib.rs.
Every unit test that reads or writes a process-global env var now serializes
behind that one lock, regardless of module. Production code is unchanged;
this is a test-hygiene fix. main.rs keeps its own guard (separate test
binary, already internally consistent).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol merged commit ce62e51 into main May 24, 2026
@zzylol
zzylol deleted the fix/control-plane-env-race branch July 17, 2026 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant