fix(plugins): sweep temp files a killed process left behind
Review finding, and the adjacent case to the one this PR already fixed. _sweep_debris collects *.partial and *.corrupt; _write_identity and _install_salt both create telemetry-*.<pid>.tmp and unlink it in a finally, which a SIGKILL skips. Its own docstring reasoning, that no glob in the module matches them so nothing else ever will, applies equally. Collected on the stale window rather than the expiry window: unlike a quarantined batch a temp file carries nothing worth keeping for diagnosis. 276 passed, 8 skipped. Claude-Session: https://claude.ai/code/session_01C7tEmH86HAr7GoAAKCEHZb
This commit is contained in:
@@ -486,6 +486,15 @@ def _sweep_debris(directory: Path) -> None:
|
||||
quarantined.unlink()
|
||||
except OSError:
|
||||
continue
|
||||
# The same reasoning covers *.tmp. _write_identity and _install_salt both
|
||||
# create one and unlink it in a finally, which a SIGKILL skips, and no glob
|
||||
# in this module matches the leftovers either.
|
||||
for temporary in directory.glob("telemetry-*.tmp"):
|
||||
try:
|
||||
if now - temporary.stat().st_mtime > CLAIM_STALE_SECONDS:
|
||||
temporary.unlink()
|
||||
except OSError:
|
||||
continue
|
||||
|
||||
|
||||
def _claim_parked(directory: Path) -> Path | None:
|
||||
|
||||
@@ -430,3 +430,17 @@ def test_quarantined_batches_are_eventually_collected(telemetry):
|
||||
|
||||
assert fresh.exists(), "a recent quarantine was discarded before anyone could look at it"
|
||||
assert not old.exists(), "an expired quarantine was left on disk forever"
|
||||
|
||||
|
||||
def test_temp_files_orphaned_by_a_kill_are_collected(telemetry):
|
||||
"""_write_identity and _install_salt unlink in a finally, which SIGKILL skips."""
|
||||
directory = telemetry.memory_core.data_dir()
|
||||
directory.mkdir(parents=True, exist_ok=True)
|
||||
orphan = directory / "telemetry-salt.999.tmp"
|
||||
orphan.write_text("abandoned", encoding="utf-8")
|
||||
stale = time.time() - (telemetry.CLAIM_STALE_SECONDS + 60)
|
||||
os.utime(orphan, (stale, stale))
|
||||
|
||||
telemetry._sweep_debris(directory)
|
||||
|
||||
assert not orphan.exists(), "a killed process left a temp file on disk forever"
|
||||
|
||||
@@ -486,6 +486,15 @@ def _sweep_debris(directory: Path) -> None:
|
||||
quarantined.unlink()
|
||||
except OSError:
|
||||
continue
|
||||
# The same reasoning covers *.tmp. _write_identity and _install_salt both
|
||||
# create one and unlink it in a finally, which a SIGKILL skips, and no glob
|
||||
# in this module matches the leftovers either.
|
||||
for temporary in directory.glob("telemetry-*.tmp"):
|
||||
try:
|
||||
if now - temporary.stat().st_mtime > CLAIM_STALE_SECONDS:
|
||||
temporary.unlink()
|
||||
except OSError:
|
||||
continue
|
||||
|
||||
|
||||
def _claim_parked(directory: Path) -> Path | None:
|
||||
|
||||
@@ -486,6 +486,15 @@ def _sweep_debris(directory: Path) -> None:
|
||||
quarantined.unlink()
|
||||
except OSError:
|
||||
continue
|
||||
# The same reasoning covers *.tmp. _write_identity and _install_salt both
|
||||
# create one and unlink it in a finally, which a SIGKILL skips, and no glob
|
||||
# in this module matches the leftovers either.
|
||||
for temporary in directory.glob("telemetry-*.tmp"):
|
||||
try:
|
||||
if now - temporary.stat().st_mtime > CLAIM_STALE_SECONDS:
|
||||
temporary.unlink()
|
||||
except OSError:
|
||||
continue
|
||||
|
||||
|
||||
def _claim_parked(directory: Path) -> Path | None:
|
||||
|
||||
@@ -486,6 +486,15 @@ def _sweep_debris(directory: Path) -> None:
|
||||
quarantined.unlink()
|
||||
except OSError:
|
||||
continue
|
||||
# The same reasoning covers *.tmp. _write_identity and _install_salt both
|
||||
# create one and unlink it in a finally, which a SIGKILL skips, and no glob
|
||||
# in this module matches the leftovers either.
|
||||
for temporary in directory.glob("telemetry-*.tmp"):
|
||||
try:
|
||||
if now - temporary.stat().st_mtime > CLAIM_STALE_SECONDS:
|
||||
temporary.unlink()
|
||||
except OSError:
|
||||
continue
|
||||
|
||||
|
||||
def _claim_parked(directory: Path) -> Path | None:
|
||||
|
||||
@@ -486,6 +486,15 @@ def _sweep_debris(directory: Path) -> None:
|
||||
quarantined.unlink()
|
||||
except OSError:
|
||||
continue
|
||||
# The same reasoning covers *.tmp. _write_identity and _install_salt both
|
||||
# create one and unlink it in a finally, which a SIGKILL skips, and no glob
|
||||
# in this module matches the leftovers either.
|
||||
for temporary in directory.glob("telemetry-*.tmp"):
|
||||
try:
|
||||
if now - temporary.stat().st_mtime > CLAIM_STALE_SECONDS:
|
||||
temporary.unlink()
|
||||
except OSError:
|
||||
continue
|
||||
|
||||
|
||||
def _claim_parked(directory: Path) -> Path | None:
|
||||
|
||||
@@ -486,6 +486,15 @@ def _sweep_debris(directory: Path) -> None:
|
||||
quarantined.unlink()
|
||||
except OSError:
|
||||
continue
|
||||
# The same reasoning covers *.tmp. _write_identity and _install_salt both
|
||||
# create one and unlink it in a finally, which a SIGKILL skips, and no glob
|
||||
# in this module matches the leftovers either.
|
||||
for temporary in directory.glob("telemetry-*.tmp"):
|
||||
try:
|
||||
if now - temporary.stat().st_mtime > CLAIM_STALE_SECONDS:
|
||||
temporary.unlink()
|
||||
except OSError:
|
||||
continue
|
||||
|
||||
|
||||
def _claim_parked(directory: Path) -> Path | None:
|
||||
|
||||
@@ -486,6 +486,15 @@ def _sweep_debris(directory: Path) -> None:
|
||||
quarantined.unlink()
|
||||
except OSError:
|
||||
continue
|
||||
# The same reasoning covers *.tmp. _write_identity and _install_salt both
|
||||
# create one and unlink it in a finally, which a SIGKILL skips, and no glob
|
||||
# in this module matches the leftovers either.
|
||||
for temporary in directory.glob("telemetry-*.tmp"):
|
||||
try:
|
||||
if now - temporary.stat().st_mtime > CLAIM_STALE_SECONDS:
|
||||
temporary.unlink()
|
||||
except OSError:
|
||||
continue
|
||||
|
||||
|
||||
def _claim_parked(directory: Path) -> Path | None:
|
||||
|
||||
Reference in New Issue
Block a user