Skip to content

Commit 3829d81

Browse files
ztsalexeyclaude
andauthored
fix: consolidate per-module ENV_MUTEX into crate-wide test lock (#246)
Each config test module (llm.rs, embeddings.rs) defined its own ENV_MUTEX, which doesn't prevent cross-module env races since cargo test runs in parallel. Move to a single shared mutex in config/helpers.rs so all unsafe set_var/remove_var calls are serialized crate-wide. Closes #245 Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 8a4f3b6 commit 3829d81

3 files changed

Lines changed: 12 additions & 10 deletions

File tree

src/config/embeddings.rs

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -102,16 +102,12 @@ impl EmbeddingsConfig {
102102
#[cfg(test)]
103103
mod tests {
104104
use super::*;
105+
use crate::config::helpers::ENV_MUTEX;
105106
use crate::settings::{EmbeddingsSettings, Settings};
106-
use std::sync::Mutex;
107-
108-
/// Serializes env-mutating tests to prevent parallel races.
109-
static ENV_MUTEX: Mutex<()> = Mutex::new(());
110107

111108
/// Clear all embedding-related env vars.
112109
fn clear_embedding_env() {
113-
// SAFETY: Only called under ENV_MUTEX in tests. No other threads
114-
// observe these vars while the lock is held.
110+
// SAFETY: Only called under ENV_MUTEX in tests.
115111
unsafe {
116112
std::env::remove_var("EMBEDDING_ENABLED");
117113
std::env::remove_var("EMBEDDING_PROVIDER");

src/config/helpers.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,15 @@ use crate::error::ConfigError;
22

33
use super::INJECTED_VARS;
44

5+
/// Crate-wide mutex for tests that mutate process environment variables.
6+
///
7+
/// The process environment is global state shared across all threads.
8+
/// Per-module mutexes do NOT prevent races between modules running in
9+
/// parallel. Every `unsafe { set_var / remove_var }` call in tests
10+
/// MUST hold this single lock.
11+
#[cfg(test)]
12+
pub(crate) static ENV_MUTEX: std::sync::Mutex<()> = std::sync::Mutex::new(());
13+
514
pub(crate) fn optional_env(key: &str) -> Result<Option<String>, ConfigError> {
615
// Check real env vars first (always win over injected secrets)
716
match std::env::var(key) {

src/config/llm.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -383,11 +383,8 @@ fn default_session_path() -> PathBuf {
383383
#[cfg(test)]
384384
mod tests {
385385
use super::*;
386+
use crate::config::helpers::ENV_MUTEX;
386387
use crate::settings::Settings;
387-
use std::sync::Mutex;
388-
389-
/// Serializes env-mutating tests to prevent parallel races.
390-
static ENV_MUTEX: Mutex<()> = Mutex::new(());
391388

392389
/// Clear all openai-compatible-related env vars.
393390
fn clear_openai_compatible_env() {

0 commit comments

Comments
 (0)