Repository navigation
fix(process): retire epoch records whose pid was reused - #209
Merged
Merged
Conversation
A stale core-<epoch>.pid survives an unclean exit. Once its pid names another process, the start token differs, which proves the recorded core exited. Reaping treated that as unproven ownership and returned an error, so the manager's startup sweep failed and the host could not start until the record was deleted by hand (clash-nyanpasu#6033). Decide reuse by the start token alone and retire such a record without signaling the process or its descendants. A matching start token with a different or unreadable executable stays an error. Reading another user's process could not reach that decision. Linux denies /proc/<pid>/exe across users, so the executable becomes optional and the token is read from the world-readable stat file. macOS reports a start time of 0 through sysinfo when PROC_PIDTBSDINFO is refused, so the token now comes from sysctl(KERN_PROC_PID), which reports every uid and the same p_start seconds. libc does not bind kinfo_proc; the module declares the prefix it reads and checks its offsets at compile time. On Linux an executable replaced after exec reads "<name> (deleted)"; with an equal start token it still matches, so an orphan core left by an upgrade is reaped instead of blocking startup. CI now runs the process identity tests, which it previously only compiled.
|
Review changes with Changed Files
|
tokio::fs::File completes writes on a background thread, so returning after write_all can leave the truncated, empty file visible to a reader. The legacy pid file tests read it right after spawn and fail on macOS now that CI runs them.
`sh -c "sleep 30"` may exec into sleep after the test reads the child's identity. The start token stays the same while the executable changes, which correctly cannot prove ownership, so the test failed whenever the exec won the race. It surfaced on macOS once CI started running it.
Without a pre_exec hook std spawns through glibc posix_spawn, which uses clone(CLONE_VM | CLONE_VFORK). The kernel wakes the vfork parent in exec_mmap() (mm_exit_exec_release) before it installs the new mm, so spawn() can return while /proc/<pid>/exe still names the parent's binary. processkit takes that path on Linux when no writable cgroup is available, and PidFileGuard::write then failed with "executable ... does not match". Keep polling within the existing identity wait until the executable matches. The start token does not change across exec, so the identity that matches still belongs to the spawned process; a real mismatch is reported after the bounded wait as before.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the root cause of libnyanpasu/clash-nyanpasu#6033: the app panics during setup on macOS because a stale
core-<epoch>.pidpoints at a pid that now belongs to an unrelated process.Problem
reap_recordtreated a live pid whose start token differs from the record as "cannot prove ownership" and returnedPermissionDenied. A different start token means the recorded process exited and the pid was reused. The manager's startup sweep propagated the error, soCoreManagerconstruction failed on every launch until the user deleted the record by hand.Reading another user's process could not even reach that comparison:
PROC_PIDTBSDINFO, which is refused for another uid; sysinfo then reports0./proc/<pid>/exeis unreadable for another user's process, soprocess_identityreturned an error.Change
AlreadyExited) before descendants are captured, so neither the process nor its tree is signaled. A matching token with a different or unreadable executable is still an error and the record is kept.recorded_process_is_alivefollows the same three-way comparison.ProcessIdentity.executableis optional. Linux reads the token from the world-readablestatfile even whenexeis denied.sysctl(KERN_PROC_PID), which reports every uid and the samep_start.tv_secthat existing records hold.libcdoes not bindkinfo_proc; the module declares the prefix it reads, explains why in anFFI-BINDINGcomment (alternatives such asproc_pid_rusage, short BSD info and the private unique-identifier flavor were ruled out against XNU sources), and asserts its offsets at compile time.<name> (deleted)matches the recorded name, so an orphan core left by an upgrade is reaped.Tests
Verified locally on Windows and on Linux (WSL, also as
nobody). macOS was only type-checked and Clippy-checked for both architectures; its behavior tests rely on this CI.Known limitation
Windows: if the pid is reused by a protected (PPL) process,
OpenProcessmay be denied and the reap still errors.