Skip to content

fix(compile): sweep the leaked perry_strip scratch directories (#7261) - #7297

Merged
proggeramlug merged 1 commit into
mainfrom
fix/7261-strip-scratch-leak
Aug 3, 2026
Merged

fix(compile): sweep the leaked perry_strip scratch directories (#7261)#7297
proggeramlug merged 1 commit into
mainfrom
fix/7261-strip-scratch-leak

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Fixes #7261.

perry compile creates perry_strip_<pid>/ under the system temp directory at eight sites and never removes it. The _extract subdirectories are cleaned; the parent — holding every _<lib>_trimmed.lib — is not.

Why this is worth a PR rather than a periodic rm

It leaks one directory per compile, at roughly 64 per two hours of ordinary activity, and it twice took this development machine to zero bytes free today.

The failure mode is badly mis-signalled: a full disk surfaces as unrelated build and test failures in every concurrent process, not as a disk error at the leak site. It killed one background agent outright and stalled another, and the time went into debugging the symptoms.

The fix

All eight sites now share one strip_tmp_base(), which on first use creates the directory and sweeps other processes' dead-PID directories.

Startup sweep rather than an exit hook, deliberately. A Drop guard or atexit handler does not run on SIGKILL, on a panic-abort, or on process::exit — exactly the cases that leave the largest messes. A sweep at first use heals all of them, including leftovers from crashes that happened before this change landed.

A live PID's directory is never touched, so concurrent perry invocations are safe.

The bug my own boundary test caught

The first version probed liveness with libc::kill(pid as pid_t, 0). u32::MAX as pid_t is -1, and kill(-1, 0) means "every process we may signal" — it succeeds, so a dead directory read as live and was never swept. kill() gives special meaning to every non-positive pid: 0 is our process group, -1 is everything, < -1 is a process group.

pid_is_live now rejects anything that cannot fit a positive pid_t before probing. This is why the test asserts the boundaries (self, 1, u32::MAX, 0) rather than just the happy path.

Errno is read through std::io::Error::last_os_error() rather than a platform-specific symbol (__error on macOS vs __errno_location on Linux), and anything other than ESRCH errs toward "live" so an unexpected errno never deletes a directory in use.

Verification

  • 3 tests, cargo test -p perry --bins — note perry is a binary crate with no lib target, so these run in the bin harness. All pass.
  • Sabotage-checked: with the remove_dir_all removed, sweep_removes_dead_pid_dirs_and_keeps_live_ones fails with dead-PID dir must be swept (#7261). The test has teeth.
  • The live-PID control (perry_strip_1, init/launchd) proves the sweep discriminates rather than deleting everything it finds.
  • All four lint gates pass locally, plus cargo fmt --check.

Not verified

  • Unix only. pid_is_live returns true unconditionally on non-Unix, so Windows keeps the current behaviour (leak) rather than risking a wrong probe. Worth a follow-up if Windows compiles are affected.
  • I did not add a removal at the end of a compile. The sweep makes the steady state at most one directory per live process, which is the property that matters; an end-of-compile removal is a possible refinement, not a correctness requirement.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed perry compile leaving behind temporary directories after each invocation, which could eventually exhaust disk space.
    • Temporary files are now cleaned up safely at startup while preserving directories still used by active processes.
    • Improved handling of abnormal termination and platform-specific process detection.

@proggeramlug
proggeramlug merged commit 4bfbf52 into main Aug 3, 2026
29 of 39 checks passed
@proggeramlug
proggeramlug deleted the fix/7261-strip-scratch-leak branch August 3, 2026 08:17
@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 392ed099-f7c6-4c82-90d5-b0b7f2338154

📥 Commits

Reviewing files that changed from the base of the PR and between cdc7dee and 55efa1e.

📒 Files selected for processing (3)
  • changelog.d/7296-strip-scratch-leak.md
  • crates/perry/src/commands/compile/strip_dedup.rs
  • crates/perry/src/commands/compile/strip_dedup/stub_symbols.rs

📝 Walkthrough

Walkthrough

perry compile now shares one strip scratch directory per process. Startup cleanup removes directories for dead PIDs while preserving live-process directories. All archive-stripping paths use the shared helper, and tests cover cleanup and PID handling.

Changes

Strip scratch cleanup

Layer / File(s) Summary
Temporary-directory manager and cleanup tests
crates/perry/src/commands/compile/strip_dedup.rs, changelog.d/7296-strip-scratch-leak.md
The shared helper initializes one process directory, removes stale dead-PID directories, preserves live directories, and documents the cleanup behavior. Tests cover initialization and PID liveness.
Archive stripping integration
crates/perry/src/commands/compile/strip_dedup.rs
Archive trimming and library localization paths use the shared temporary-directory manager.
Stub and reference archive integration
crates/perry/src/commands/compile/strip_dedup/stub_symbols.rs
Windows stub stripping, Unix stub localization, and reference-member rebuilding use the shared temporary-directory manager.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

  • PerryTS/perry#7085: Both changes modify archive-stripping and deduplication workflows in the same modules.

Suggested labels: bug, tooling

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/7261-strip-scratch-leak

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

perry compile leaks its perry_strip_<pid> temp dir on every run (filled a dev volume to 0 bytes)

1 participant