diff --git a/apps/bootstrap-installer/src-tauri/src/marker.rs b/apps/bootstrap-installer/src-tauri/src/marker.rs index 027ce32817..10a43af694 100644 --- a/apps/bootstrap-installer/src-tauri/src/marker.rs +++ b/apps/bootstrap-installer/src-tauri/src/marker.rs @@ -912,13 +912,34 @@ fn sweep_tmp_litter(path: &Path) { continue; }; let pid = rest.split('.').next().unwrap_or(""); - let dead = is_ascii_digits(pid) && !pid.parse::().is_ok_and(pid_is_alive); - if dead { + if !is_ascii_digits(pid) { + continue; + } + let gone = match pid.parse::() { + Ok(pid) if pid == std::process::id() => own_pid_tmp_is_stale(&entry), + Ok(pid) => !pid_is_alive(pid), + Err(_) => true, // past u32: no process has that pid + }; + if gone { let _ = std::fs::remove_file(entry.path()); } } } +/// Every tmp write of ours publishes or is removed well inside this: an older +/// tmp under our own pid number is a previous holder's (containers reuse pids +/// every boot), never ours in flight. Mirrors `OWN_PID_TMP_STALE_SECONDS`. +const OWN_PID_TMP_STALE: Duration = Duration::from_secs(60); + +fn own_pid_tmp_is_stale(entry: &std::fs::DirEntry) -> bool { + entry + .metadata() + .and_then(|meta| meta.modified()) + .ok() + .and_then(|modified| modified.elapsed().ok()) + .is_some_and(|age| age > OWN_PID_TMP_STALE) +} + /// Outcome of one claim attempt. enum Publish { Claimed, diff --git a/apps/bootstrap-installer/src-tauri/src/marker_tests.rs b/apps/bootstrap-installer/src-tauri/src/marker_tests.rs index ea84c39711..e5b7691ff0 100644 --- a/apps/bootstrap-installer/src-tauri/src/marker_tests.rs +++ b/apps/bootstrap-installer/src-tauri/src/marker_tests.rs @@ -1180,6 +1180,29 @@ fn claim_sweeps_dead_claimants_tmp_litter() { let _ = std::fs::remove_dir_all(&dir); } +#[test] +fn stale_tmp_under_our_own_pid_number_is_swept() { + // Containers reuse pid numbers every boot: a tmp a previous holder of our pid left is + // litter once it is older than any write of ours; a fresh one may be in flight. + let dir = unique_tmp_dir("marker-own-pid-litter"); + let marker = dir.join(".hermes-update-in-progress"); + let me = std::process::id(); + let stale = dir.join(format!(".hermes-update-in-progress.{me}.deadbeef.tmp")); + let fresh = dir.join(format!(".hermes-update-in-progress.{me}.cafe.tmp")); + std::fs::write(&fresh, "").unwrap(); + let file = std::fs::File::create(&stale).unwrap(); + file.set_modified(SystemTime::now() - Duration::from_secs(3600)) + .unwrap(); + drop(file); + sweep_tmp_litter(&marker); + assert!( + !stale.exists(), + "a stale tmp under our pid number is litter" + ); + assert!(fresh.exists(), "a fresh tmp under our pid may be in flight"); + let _ = std::fs::remove_dir_all(&dir); +} + /// The live owner a refused acquire reports; panics on any other outcome. fn busy(result: Result) -> MarkerOwner { match result { diff --git a/hermes_cli/update_lock.py b/hermes_cli/update_lock.py index a096eb0bbc..67d474abd7 100644 --- a/hermes_cli/update_lock.py +++ b/hermes_cli/update_lock.py @@ -668,6 +668,19 @@ def _tmp_sibling(path: Path) -> Path: return path.with_name(f"{path.name}.{os.getpid()}.{secrets.token_hex(4)}.tmp") +# Every tmp write of ours publishes or is unlinked well inside this: an older tmp under our own +# pid number is a previous holder's (containers reuse pids every boot), never ours in flight. +OWN_PID_TMP_STALE_SECONDS = 60.0 + + +def _tmp_writer_gone(owner: int | None, entry: Path) -> bool: + if owner is None: + return True # ASCII digits past u32: no process has that pid + if owner != os.getpid(): + return not _pid_alive(owner) + return time.time() - entry.stat().st_mtime > OWN_PID_TMP_STALE_SECONDS + + def _sweep_dead_tmp_siblings(path: Path) -> None: """Reclaim ``.[.].tmp`` files whose writer died between write and publish (contract m10): the pid is the first component after the marker name.""" @@ -675,11 +688,11 @@ def _sweep_dead_tmp_siblings(path: Path) -> None: with suppress(OSError): for entry in path.parent.iterdir(): name = entry.name - if not (name.startswith(prefix) and name.endswith(".tmp")): + owner = name[len(prefix):].split(".", 1)[0] + if not (name.startswith(prefix) and name.endswith(".tmp") and _INT_LINE.fullmatch(owner)): continue - owner = _bounded_int(name[len(prefix):].split(".", 1)[0], _U32_MAX) - if owner is not None and owner != os.getpid() and not _pid_alive(owner): - with suppress(OSError): + with suppress(OSError): + if _tmp_writer_gone(_bounded_int(owner, _U32_MAX), entry): entry.unlink() diff --git a/tests/hermes_cli/test_update_lock.py b/tests/hermes_cli/test_update_lock.py index dfa59c6c4e..58e9dbddc0 100644 --- a/tests/hermes_cli/test_update_lock.py +++ b/tests/hermes_cli/test_update_lock.py @@ -329,6 +329,23 @@ def test_claim_publishes_whole_and_reclaims_dead_writers_tmp_files(marker): lock.release() +def test_stale_tmp_under_our_own_pid_number_is_reclaimed(marker): + """Containers hand out the same pids every boot: a tmp a previous holder of our pid number + left is litter once it is older than any write of ours, while a fresh one may be in flight.""" + stale = marker.with_name(f"{marker.name}.{os.getpid()}.deadbeef.tmp") + fresh = marker.with_name(f"{marker.name}.{os.getpid()}.cafe.tmp") + stale.write_text("x") + fresh.write_text("x") + hour_ago = time.time() - 3600 + os.utime(stale, (hour_ago, hour_ago)) + + lock = UpdateLock(path=marker) + assert lock.acquire() is True + lock.release() + assert not stale.exists() + assert fresh.exists() + + @pytest.mark.parametrize("owner", ["\u00b2", "\u2460"], ids=["superscript-two", "circled-one"]) def test_tmp_litter_with_a_digit_lookalike_pid_never_breaks_admission(marker, owner): """A sibling whose pid field is a Unicode digit lookalike is not a pid we can judge: it is