Skip to content

Commit 6116c6f

Browse files
committed
fix: handle temp file removal race condition in concurrent initialization
When multiple SQLPage instances start simultaneously in the same directory, they can encounter a race condition during initialization. The create_default_database() function creates a temporary file to test directory writability, then removes it. If multiple instances try to remove the same file concurrently, some panic with 'No such file or directory'. This commit replaces the .expect() panic with graceful error handling using if let Err(). The writability test has already succeeded by the time we try to remove the file, so whether another instance removed it is irrelevant. Includes a test that spawns 10 concurrent threads initializing AppConfig to verify no panics occur. ref #1183
1 parent 8d106fb commit 6116c6f

1 file changed

Lines changed: 45 additions & 1 deletion

File tree

src/app_config.rs

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -495,7 +495,11 @@ fn create_default_database(configuration_directory: &Path) -> String {
495495
default_db_path.display()
496496
);
497497
drop(tmp_file);
498-
std::fs::remove_file(&default_db_path).expect("removing temp file");
498+
// Gracefully handle removal - file might already be removed by another instance
499+
// in concurrent startup scenarios.
500+
if let Err(e) = std::fs::remove_file(&default_db_path) {
501+
log::debug!("Temp file already removed or not found: {}", e);
502+
}
499503
return prefix + &encode_uri(&default_db_path) + "?mode=rwc";
500504
}
501505
}
@@ -798,4 +802,44 @@ mod test {
798802
"Configuration directory should default to ./sqlpage when not specified"
799803
);
800804
}
805+
806+
#[test]
807+
fn test_concurrent_initialization_no_panic() {
808+
let _lock = ENV_LOCK
809+
.lock()
810+
.expect("Another test panicked while holding the lock");
811+
812+
let test_dir = std::env::temp_dir().join(format!(
813+
"sqlpage_race_test_{}",
814+
std::time::SystemTime::now()
815+
.duration_since(std::time::UNIX_EPOCH)
816+
.unwrap()
817+
.as_nanos()
818+
));
819+
std::fs::create_dir_all(&test_dir).unwrap();
820+
821+
env::remove_var("DATABASE_URL");
822+
823+
let handles: Vec<_> = (0..10)
824+
.map(|_| {
825+
let test_dir_clone = test_dir.clone();
826+
std::thread::spawn(move || {
827+
let cli = Cli {
828+
web_root: None,
829+
config_dir: Some(test_dir_clone),
830+
config_file: None,
831+
command: None,
832+
};
833+
let result = AppConfig::from_cli(&cli);
834+
assert!(result.is_ok(), "AppConfig initialization should succeed");
835+
})
836+
})
837+
.collect();
838+
839+
for handle in handles {
840+
handle.join().expect("Thread should not panic");
841+
}
842+
843+
let _ = std::fs::remove_dir_all(&test_dir);
844+
}
801845
}

0 commit comments

Comments
 (0)