From 6116c6fa1dc7086a4633be8ac45cb393bad359d7 Mon Sep 17 00:00:00 2001 From: mukhtaronif Date: Sun, 11 Jan 2026 18:12:55 +0000 Subject: [PATCH 1/4] 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 --- src/app_config.rs | 46 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/src/app_config.rs b/src/app_config.rs index d4532c5b..9f24b924 100644 --- a/src/app_config.rs +++ b/src/app_config.rs @@ -495,7 +495,11 @@ fn create_default_database(configuration_directory: &Path) -> String { default_db_path.display() ); drop(tmp_file); - std::fs::remove_file(&default_db_path).expect("removing temp file"); + // Gracefully handle removal - file might already be removed by another instance + // in concurrent startup scenarios. + if let Err(e) = std::fs::remove_file(&default_db_path) { + log::debug!("Temp file already removed or not found: {}", e); + } return prefix + &encode_uri(&default_db_path) + "?mode=rwc"; } } @@ -798,4 +802,44 @@ mod test { "Configuration directory should default to ./sqlpage when not specified" ); } + + #[test] + fn test_concurrent_initialization_no_panic() { + let _lock = ENV_LOCK + .lock() + .expect("Another test panicked while holding the lock"); + + let test_dir = std::env::temp_dir().join(format!( + "sqlpage_race_test_{}", + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + std::fs::create_dir_all(&test_dir).unwrap(); + + env::remove_var("DATABASE_URL"); + + let handles: Vec<_> = (0..10) + .map(|_| { + let test_dir_clone = test_dir.clone(); + std::thread::spawn(move || { + let cli = Cli { + web_root: None, + config_dir: Some(test_dir_clone), + config_file: None, + command: None, + }; + let result = AppConfig::from_cli(&cli); + assert!(result.is_ok(), "AppConfig initialization should succeed"); + }) + }) + .collect(); + + for handle in handles { + handle.join().expect("Thread should not panic"); + } + + let _ = std::fs::remove_dir_all(&test_dir); + } } From 31e7b0f954cf4975bc372baafed518110f9bfd55 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Sun, 11 Jan 2026 21:54:10 +0100 Subject: [PATCH 2/4] cargo fmt --- src/app_config.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app_config.rs b/src/app_config.rs index 9f24b924..5998e588 100644 --- a/src/app_config.rs +++ b/src/app_config.rs @@ -496,7 +496,7 @@ fn create_default_database(configuration_directory: &Path) -> String { ); drop(tmp_file); // Gracefully handle removal - file might already be removed by another instance - // in concurrent startup scenarios. + // in concurrent startup scenarios. if let Err(e) = std::fs::remove_file(&default_db_path) { log::debug!("Temp file already removed or not found: {}", e); } From 9b6f8d1da08bcb9643a626061afcbcd47e01e713 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Sun, 11 Jan 2026 22:49:03 +0100 Subject: [PATCH 3/4] the error may have another cause --- src/app_config.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/app_config.rs b/src/app_config.rs index 5998e588..8372bf03 100644 --- a/src/app_config.rs +++ b/src/app_config.rs @@ -495,10 +495,8 @@ fn create_default_database(configuration_directory: &Path) -> String { default_db_path.display() ); drop(tmp_file); - // Gracefully handle removal - file might already be removed by another instance - // in concurrent startup scenarios. if let Err(e) = std::fs::remove_file(&default_db_path) { - log::debug!("Temp file already removed or not found: {}", e); + log::debug!("Unable to remove temporary probe file. It might have already been removed by another instance started concurrently: {}", e); } return prefix + &encode_uri(&default_db_path) + "?mode=rwc"; } From 85012186ca3da1f8142baf9c7df127849d9a9c09 Mon Sep 17 00:00:00 2001 From: Ophir LOJKINE Date: Mon, 12 Jan 2026 12:14:32 +0100 Subject: [PATCH 4/4] Remove concurrent initialization test Removed the test for concurrent initialization. The test did not work --- src/app_config.rs | 40 ---------------------------------------- 1 file changed, 40 deletions(-) diff --git a/src/app_config.rs b/src/app_config.rs index 8372bf03..b28150ca 100644 --- a/src/app_config.rs +++ b/src/app_config.rs @@ -800,44 +800,4 @@ mod test { "Configuration directory should default to ./sqlpage when not specified" ); } - - #[test] - fn test_concurrent_initialization_no_panic() { - let _lock = ENV_LOCK - .lock() - .expect("Another test panicked while holding the lock"); - - let test_dir = std::env::temp_dir().join(format!( - "sqlpage_race_test_{}", - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_nanos() - )); - std::fs::create_dir_all(&test_dir).unwrap(); - - env::remove_var("DATABASE_URL"); - - let handles: Vec<_> = (0..10) - .map(|_| { - let test_dir_clone = test_dir.clone(); - std::thread::spawn(move || { - let cli = Cli { - web_root: None, - config_dir: Some(test_dir_clone), - config_file: None, - command: None, - }; - let result = AppConfig::from_cli(&cli); - assert!(result.is_ok(), "AppConfig initialization should succeed"); - }) - }) - .collect(); - - for handle in handles { - handle.join().expect("Thread should not panic"); - } - - let _ = std::fs::remove_dir_all(&test_dir); - } }