diff --git a/apps/effectcraft/src/crash_report.rs b/apps/effectcraft/src/crash_report.rs index 837161811..32306aa5c 100644 --- a/apps/effectcraft/src/crash_report.rs +++ b/apps/effectcraft/src/crash_report.rs @@ -182,7 +182,7 @@ fn clean_exit(input: &mut impl Read) -> io::Result { } enum SessionLog { - Captured(File), + Captured { file: File, marker: String }, Unavailable(String), } @@ -201,7 +201,7 @@ impl SessionLog { Ok(file) })(); match opened { - Ok(file) => Self::Captured(file), + Ok(file) => Self::Captured { file, marker: format!("{}\n", session_marker(id)) }, Err(e) => Self::Unavailable(e.to_string()), } } @@ -209,7 +209,15 @@ impl SessionLog { fn write_tail(&mut self, report: &mut File) -> io::Result<()> { match self { Self::Unavailable(reason) => writeln!(report, "Recent log unavailable: {reason}"), - Self::Captured(file) => { + Self::Captured { file, marker } => { + // An older concurrent launcher may have truncated the captured file. + // Recheck its identity before copying any content into this report. + file.seek(SeekFrom::Start(0))?; + let mut header = [0u8; HEADER_BYTES]; + let prefix = header.get_mut(..marker.len()).ok_or_else(|| io::Error::other("session marker too long"))?; + if file.read_exact(prefix).is_err() || prefix != marker.as_bytes() { + return writeln!(report, "Recent log unavailable: session identity changed"); + } let size = file.metadata()?.len(); file.seek(SeekFrom::Start(size.saturating_sub(LOG_BYTES as u64)))?; writeln!(report, "\nRecorded session log tail (last {LOG_BYTES} bytes at most; logging filters/size caps may omit later events):")?; @@ -350,6 +358,21 @@ mod tests { std::fs::remove_dir_all(dir).unwrap(); } + #[test] + fn captured_log_rejects_a_session_overwritten_after_open() { + let dir = folder(); + let path = dir.join("log"); + std::fs::write(&path, "crash-report session 1-2\noriginal session\n").unwrap(); + let mut log = SessionLog::open(Some(&path), "1-2"); + std::fs::write(&path, "crash-report session 3-4\nprivate other session\n").unwrap(); + let report = write_report(&dir, "1-2", &mut log, None).unwrap(); + let text = std::fs::read_to_string(report).unwrap(); + assert!(text.contains("Recent log unavailable")); + assert!(!text.contains("private other session")); + assert!(!text.contains("session 3-4")); + std::fs::remove_dir_all(dir).unwrap(); + } + #[test] fn session_identity_requires_the_exact_first_line() { let dir = folder(); diff --git a/crates/engine/src/system/logging.rs b/crates/engine/src/system/logging.rs index 198aa3b46..2bb3518a1 100644 --- a/crates/engine/src/system/logging.rs +++ b/crates/engine/src/system/logging.rs @@ -214,10 +214,16 @@ fn rotate(dir: &Path) -> std::io::Result<(PathBuf, File)> { } } let path = numbered(dir, 0); - let file = File::create(&path)?; + let file = create_session_file(&path)?; Ok((path, file)) } +// Another process may create the current log after our rotation. Never truncate its +// file: its writer and crash reporter may already hold the same inode open. +fn create_session_file(path: &Path) -> std::io::Result { + std::fs::OpenOptions::new().write(true).create_new(true).open(path) +} + /// Rotate the session logs in `dir` (the `Logs` folder, see [`log_dir`]) and write records to /// a fresh `effectcraft.log` there, first the ones logged so far; returns its path. pub fn attach_session_log(dir: &Path) -> Result { @@ -428,6 +434,20 @@ mod tests { assert_eq!(timestamp(u64::MAX), "584556019-04-03T14:25:51.615Z"); } + #[test] + fn concurrent_session_creation_does_not_truncate_the_winner() { + let dir = temp_dir("concurrent-create"); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join(SESSION_LOG); + let mut winner = create_session_file(&path).unwrap(); + winner.write_all(b"crash-report session 1-2\nwinner content\n").unwrap(); + assert_eq!(create_session_file(&path).unwrap_err().kind(), std::io::ErrorKind::AlreadyExists); + winner.write_all(b"still the same session\n").unwrap(); + assert_eq!(read(&path), "crash-report session 1-2\nwinner content\nstill the same session\n"); + drop(winner); + std::fs::remove_dir_all(dir).unwrap(); + } + /// Each launch keeps the previous runs' logs, so the log of a run that crashed survives. #[test] fn rotation_keeps_the_previous_logs_and_starts_an_empty_file() {