fix(update): tmp litter under our own reused pid number is reclaimed once stale (review PID-reuse-tmp-sweep)
What: the Python and Rust tmp sweeps reclaim a tmp sibling named for our own pid once its mtime is older than 60 s (OWN_PID_TMP_STALE[_SECONDS]); other pids keep the liveness check, and an ASCII pid past u32 is dead in both (Python had kept it since N06; Rust always swept it). Why: with deterministic container pids a previous holder of our pid number left a tmp that every later run skipped as 'our own in-flight write', so it was never cleaned. Every write of ours finishes far inside 60 s, so a fresh same-pid tmp is still kept. Red: test_stale_tmp_under_our_own_pid_number_is_reclaimed FAILED and marker_tests::stale_tmp_under_our_own_pid_number_is_swept FAILED on 07c014355b5 (real marker.rs via a std-only harness; the Tauri crate cannot build here without webkit2gtk/dbus). Green: both pass; test_update_lock.py 59 passed/1 skipped; marker tests 49 passed/2 ignored.
This commit is contained in:
@@ -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::<u32>().is_ok_and(pid_is_alive);
|
||||
if dead {
|
||||
if !is_ascii_digits(pid) {
|
||||
continue;
|
||||
}
|
||||
let gone = match pid.parse::<u32>() {
|
||||
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,
|
||||
|
||||
@@ -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<UpdateMarkerGuard, AcquireError>) -> MarkerOwner {
|
||||
match result {
|
||||
|
||||
@@ -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 ``<marker>.<pid>[.<token>].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()
|
||||
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user