paavohtl Claude Opus 4.6 (1M context) commited on
Commit
10cb6f9
·
1 Parent(s): bb59078

fix: high-concurrency test crashes from premature thread resume

Browse files

Root cause: the launcher's hooks-ready event had a 10s timeout. Under
high concurrency (j=16), DLL loading + hook installation exceeds 10s
due to I/O contention, causing the launcher to resume WA.exe's main
thread before all hooks are installed. The FireWeapon trap then fires
because WeaponRelease (last hook in the chain) was never installed.

Fix: increase hooks-ready timeout to 120s (matching test runner timeout).

Also:
- Redirect all temp files to per-PID .openwa_tmp/{pid}/ directory
instead of individual per-PID suffixed filenames. Now covers
writetest.txt, mono.tmp, custom.dat, current.thm, playback.thm
in addition to existing land.dat, landgen.svg
- Clean up temp directory on DLL detach and in test runner
- Remove 200ms test stagger (no longer needed with proper isolation)
- Add .gitignore for replay output logs (only *_expected.log tracked)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

crates/openwa-launcher/src/main.rs CHANGED
@@ -230,9 +230,11 @@ unsafe fn launch(
230
  }
231
 
232
  // Wait for the DLL to signal that all hooks are installed.
233
- // Timeout after 10s to avoid hanging if the DLL crashes during init.
 
 
234
  if !event.is_null() {
235
- let wait_result = WaitForSingleObject(event, 10_000);
236
  if wait_result != 0 {
237
  eprintln!("openwa-launcher: warning: hooks-ready event timed out ({wait_result})");
238
  }
 
230
  }
231
 
232
  // Wait for the DLL to signal that all hooks are installed.
233
+ // Use a generous timeout — under high concurrency (e.g. 16 parallel
234
+ // test instances), DLL loading + hook installation can exceed 10s due
235
+ // to I/O contention. 120s matches the test runner's own timeout.
236
  if !event.is_null() {
237
+ let wait_result = WaitForSingleObject(event, 120_000);
238
  if wait_result != 0 {
239
  eprintln!("openwa-launcher: warning: hooks-ready event timed out ({wait_result})");
240
  }
crates/openwa-test-runner/src/main.rs CHANGED
@@ -433,12 +433,7 @@ fn run_tests_parallel(
433
  }
434
  drop(tx); // Drop our sender so rx closes when all workers finish
435
 
436
- // Submit work with small stagger to reduce startup races from unidentified
437
- // shared resources (writetest.txt, steam.dat, or unknown Win32 globals).
438
  for (idx, test) in tests.into_iter().enumerate() {
439
- if idx > 0 {
440
- thread::sleep(Duration::from_millis(200));
441
- }
442
  let _ = work_tx.send((idx, test));
443
  }
444
  drop(work_tx); // Signal no more work
@@ -605,27 +600,20 @@ fn write_summary(results: &[TestResult], wall_time: Duration, path: &Path) {
605
 
606
  // ─── Helpers ────────────────────────────────────────────────────────────────
607
 
608
- /// Clean up per-PID temp files left by the file isolation hook.
609
- /// Matches patterns: mono_*.tmp, land_*.dat, landgen_*.svg, playback_*.thm, cur_*.thm
 
 
610
  fn cleanup_temp_files(wa_exe: &Path) {
611
  let game_dir = match wa_exe.parent() {
612
  Some(d) => d,
613
  None => return,
614
  };
615
 
616
- let patterns = [(game_dir, "mono_", ".tmp"), (game_dir, "cur_", ".thm")];
617
- let data_dir = game_dir.join("DATA");
618
- let data_patterns = [
619
- ("land_", ".dat"),
620
- ("landgen_", ".svg"),
621
- ("playback_", ".thm"),
622
- ];
623
-
624
- for (dir, prefix, suffix) in &patterns {
625
- cleanup_matching(dir, prefix, suffix);
626
- }
627
- for (prefix, suffix) in &data_patterns {
628
- cleanup_matching(&data_dir, prefix, suffix);
629
  }
630
 
631
  // Clean up any stale ERRORLOG.TXT / CRASH.DMP in the game directory
@@ -633,18 +621,6 @@ fn cleanup_temp_files(wa_exe: &Path) {
633
  let _ = fs::remove_file(game_dir.join("CRASH.DMP"));
634
  }
635
 
636
- fn cleanup_matching(dir: &Path, prefix: &str, suffix: &str) {
637
- if let Ok(entries) = fs::read_dir(dir) {
638
- for entry in entries.flatten() {
639
- let name = entry.file_name();
640
- let name = name.to_string_lossy();
641
- if name.starts_with(prefix) && name.ends_with(suffix) {
642
- let _ = fs::remove_file(entry.path());
643
- }
644
- }
645
- }
646
- }
647
-
648
  /// Normalize CRLF to LF for cross-platform comparison.
649
  fn normalize_crlf(data: &[u8]) -> Vec<u8> {
650
  let mut out = Vec::with_capacity(data.len());
 
433
  }
434
  drop(tx); // Drop our sender so rx closes when all workers finish
435
 
 
 
436
  for (idx, test) in tests.into_iter().enumerate() {
 
 
 
437
  let _ = work_tx.send((idx, test));
438
  }
439
  drop(work_tx); // Signal no more work
 
600
 
601
  // ─── Helpers ────────────────────────────────────────────────────────────────
602
 
603
+ /// Clean up temp files and directories left by the file isolation hook.
604
+ ///
605
+ /// The DLL creates `.openwa_tmp/{pid}/` directories and cleans them on exit,
606
+ /// but crashed processes may leave orphans. Also cleans stale ERRORLOG/CRASH files.
607
  fn cleanup_temp_files(wa_exe: &Path) {
608
  let game_dir = match wa_exe.parent() {
609
  Some(d) => d,
610
  None => return,
611
  };
612
 
613
+ // Remove the .openwa_tmp directory tree (all per-PID temp dirs)
614
+ let tmp_dir = game_dir.join(".openwa_tmp");
615
+ if tmp_dir.exists() {
616
+ let _ = fs::remove_dir_all(&tmp_dir);
 
 
 
 
 
 
 
 
 
617
  }
618
 
619
  // Clean up any stale ERRORLOG.TXT / CRASH.DMP in the game directory
 
621
  let _ = fs::remove_file(game_dir.join("CRASH.DMP"));
622
  }
623
 
 
 
 
 
 
 
 
 
 
 
 
 
624
  /// Normalize CRLF to LF for cross-platform comparison.
625
  fn normalize_crlf(data: &[u8]) -> Vec<u8> {
626
  let mut out = Vec::with_capacity(data.len());
crates/openwa-wormkit/src/replacements/file_isolation.rs CHANGED
@@ -1,13 +1,13 @@
1
  //! Per-instance file path isolation for concurrent test execution.
2
  //!
3
- //! Hooks `kernel32!CreateFileA` to redirect landscape generation temp files
4
- //! to per-PID paths, preventing races when multiple WA.exe instances run
5
  //! simultaneously. Only active when `OPENWA_HEADLESS=1` is set.
6
  //!
7
- //! Redirected files (in game directory):
8
- //! mono.tmp → mono_{pid}.tmp
9
- //! DATA\land.dat → DATA\land_{pid}.dat
10
- //! DATA\landgen.svg → DATA\landgen_{pid}.svg
11
 
12
  use crate::log_line;
13
 
@@ -33,6 +33,7 @@ type CreateFileAFn = unsafe extern "system" fn(
33
  static ORIG_CREATE_FILE_A: AtomicU32 = AtomicU32::new(0);
34
 
35
  static ERRORLOG_PATH: OnceLock<Option<String>> = OnceLock::new();
 
36
 
37
  fn errorlog_redirect() -> Option<&'static str> {
38
  ERRORLOG_PATH
@@ -40,34 +41,62 @@ fn errorlog_redirect() -> Option<&'static str> {
40
  .as_deref()
41
  }
42
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
43
  /// Check if a path ends with one of our target filenames (case-insensitive).
44
  /// Returns the replacement path if it matches, None otherwise.
45
  fn redirect_path(path: &str) -> Option<String> {
 
 
46
  // ERRORLOG.TXT → env var path (if set by test runner)
47
  if let Some(target) = errorlog_redirect() {
48
- let lower = path.to_ascii_lowercase();
49
- if lower.ends_with("\\errorlog.txt") || lower.ends_with("/errorlog.txt")
50
  || lower == "errorlog.txt"
51
  {
52
  return Some(target.to_string());
53
  }
54
  }
55
 
56
- let pid = std::process::id();
57
- let lower = path.to_ascii_lowercase();
58
 
59
- if lower.ends_with("\\mono.tmp") || lower == "mono.tmp" {
60
- let prefix = &path[..path.len() - "mono.tmp".len()];
61
- Some(format!("{prefix}mono_{pid}.tmp"))
62
- } else if lower.ends_with("\\land.dat") || lower.ends_with("/land.dat") {
63
- let prefix = &path[..path.len() - "land.dat".len()];
64
- Some(format!("{prefix}land_{pid}.dat"))
65
- } else if lower.ends_with("\\landgen.svg") || lower.ends_with("/landgen.svg") {
66
- let prefix = &path[..path.len() - "landgen.svg".len()];
67
- Some(format!("{prefix}landgen_{pid}.svg"))
68
- } else {
69
- None
70
  }
 
 
 
 
 
 
 
 
 
 
 
71
  }
72
 
73
  unsafe extern "system" fn hook_create_file_a(
@@ -135,15 +164,27 @@ pub fn install() -> Result<(), String> {
135
  ORIG_CREATE_FILE_A.store(trampoline as u32, Ordering::Relaxed);
136
 
137
  let pid = std::process::id();
138
- let errorlog_msg = if let Some(target) = errorlog_redirect() {
139
- format!(", ERRORLOG.TXT → {target}")
140
- } else {
141
- String::new()
142
- };
 
143
  let _ = log_line(&format!(
144
- "[FileIsolation] Hooked CreateFileA (pid={pid}): mono.tmp, land.dat, landgen.svg → per-PID paths{errorlog_msg}"
145
  ));
146
  }
147
 
148
  Ok(())
149
  }
 
 
 
 
 
 
 
 
 
 
 
 
1
  //! Per-instance file path isolation for concurrent test execution.
2
  //!
3
+ //! Hooks `kernel32!CreateFileA` to redirect temp/scratch files to a per-PID
4
+ //! subdirectory, preventing races when multiple WA.exe instances run
5
  //! simultaneously. Only active when `OPENWA_HEADLESS=1` is set.
6
  //!
7
+ //! Redirected files:
8
+ //! Game dir: writetest.txt, mono.tmp, custom.dat
9
+ //! DATA\: land.dat, landgen.svg, current.thm, playback.thm
10
+ //! ERRORLOG: ERRORLOG.TXT → OPENWA_ERRORLOG_PATH (if set)
11
 
12
  use crate::log_line;
13
 
 
33
  static ORIG_CREATE_FILE_A: AtomicU32 = AtomicU32::new(0);
34
 
35
  static ERRORLOG_PATH: OnceLock<Option<String>> = OnceLock::new();
36
+ static TEMP_DIR: OnceLock<Option<String>> = OnceLock::new();
37
 
38
  fn errorlog_redirect() -> Option<&'static str> {
39
  ERRORLOG_PATH
 
41
  .as_deref()
42
  }
43
 
44
+ /// Get (and lazily create) a per-PID temp directory under the game folder.
45
+ fn temp_dir() -> Option<&'static str> {
46
+ TEMP_DIR
47
+ .get_or_init(|| {
48
+ // Build path: {game_dir}\.openwa_tmp\{pid}\
49
+ let pid = std::process::id();
50
+ let mut game_dir = std::env::current_dir().ok()?;
51
+ game_dir.push(format!(".openwa_tmp\\{pid}"));
52
+ std::fs::create_dir_all(&game_dir).ok()?;
53
+ // Also create DATA subdirectory
54
+ std::fs::create_dir_all(game_dir.join("DATA")).ok()?;
55
+ Some(game_dir.to_string_lossy().into_owned())
56
+ })
57
+ .as_deref()
58
+ }
59
+
60
+ /// Files in the game root directory that need isolation.
61
+ const ROOT_FILES: &[&str] = &["writetest.txt", "mono.tmp", "custom.dat"];
62
+
63
+ /// Files in the DATA subdirectory that need isolation.
64
+ const DATA_FILES: &[&str] = &["land.dat", "landgen.svg", "current.thm", "playback.thm"];
65
+
66
  /// Check if a path ends with one of our target filenames (case-insensitive).
67
  /// Returns the replacement path if it matches, None otherwise.
68
  fn redirect_path(path: &str) -> Option<String> {
69
+ let lower = path.to_ascii_lowercase();
70
+
71
  // ERRORLOG.TXT → env var path (if set by test runner)
72
  if let Some(target) = errorlog_redirect() {
73
+ if lower.ends_with("\\errorlog.txt")
74
+ || lower.ends_with("/errorlog.txt")
75
  || lower == "errorlog.txt"
76
  {
77
  return Some(target.to_string());
78
  }
79
  }
80
 
81
+ let tmp = temp_dir()?;
 
82
 
83
+ // Check root-level files (writetest.txt, mono.tmp, custom.dat)
84
+ for &name in ROOT_FILES {
85
+ if lower.ends_with(&format!("\\{name}")) || lower == name {
86
+ return Some(format!("{tmp}\\{name}"));
87
+ }
 
 
 
 
 
 
88
  }
89
+
90
+ // Check DATA\ files (land.dat, landgen.svg, current.thm, playback.thm)
91
+ for &name in DATA_FILES {
92
+ let pattern_bs = format!("\\{name}");
93
+ let pattern_fs = format!("/{name}");
94
+ if lower.ends_with(&pattern_bs) || lower.ends_with(&pattern_fs) {
95
+ return Some(format!("{tmp}\\DATA\\{name}"));
96
+ }
97
+ }
98
+
99
+ None
100
  }
101
 
102
  unsafe extern "system" fn hook_create_file_a(
 
164
  ORIG_CREATE_FILE_A.store(trampoline as u32, Ordering::Relaxed);
165
 
166
  let pid = std::process::id();
167
+ let tmp_msg = temp_dir()
168
+ .map(|d| format!(" → {d}"))
169
+ .unwrap_or_default();
170
+ let errorlog_msg = errorlog_redirect()
171
+ .map(|t| format!(", ERRORLOG.TXT → {t}"))
172
+ .unwrap_or_default();
173
  let _ = log_line(&format!(
174
+ "[FileIsolation] Hooked CreateFileA (pid={pid}): temp files{tmp_msg}{errorlog_msg}"
175
  ));
176
  }
177
 
178
  Ok(())
179
  }
180
+
181
+ /// Clean up the per-PID temp directory. Called during DLL detach or test cleanup.
182
+ pub fn cleanup() {
183
+ if let Some(tmp) = TEMP_DIR.get().and_then(|o| o.as_deref()) {
184
+ let _ = std::fs::remove_dir_all(tmp);
185
+ // Also try to remove parent .openwa_tmp if empty
186
+ if let Some(parent) = std::path::Path::new(tmp).parent() {
187
+ let _ = std::fs::remove_dir(parent);
188
+ }
189
+ }
190
+ }
crates/openwa-wormkit/src/replacements/mod.rs CHANGED
@@ -18,9 +18,10 @@ mod team;
18
  mod weapon;
19
  mod weapon_release;
20
 
21
- /// Write gameplay milestone report. Called from DLL_PROCESS_DETACH.
22
  pub fn write_gameplay_report() {
23
  input::write_gameplay_report();
 
24
  }
25
 
26
  pub fn install_all() -> Result<(), String> {
 
18
  mod weapon;
19
  mod weapon_release;
20
 
21
+ /// Write gameplay milestone report and clean up. Called from DLL_PROCESS_DETACH.
22
  pub fn write_gameplay_report() {
23
  input::write_gameplay_report();
24
+ file_isolation::cleanup();
25
  }
26
 
27
  pub fn install_all() -> Result<(), String> {
testdata/replays/.gitignore ADDED
@@ -0,0 +1,3 @@
 
 
 
 
1
+ # Output logs from WA.exe /getlog mode — only *_expected.log baselines are tracked
2
+ *.log
3
+ !*_expected.log