Repository navigation
Detect PID reuse in detached instance state - #2025
Open
waldekmastykarz wants to merge 3 commits into
Open
waldekmastykarz wants to merge 3 commits into
waldekmastykarz wants to merge 3 commits into
Conversation
Persist the process start time in the instance state file and verify it before treating a PID as a live Dev Proxy instance. A mismatch means the OS recycled the PID and the record is stale. State files written by older versions (without the start time) keep the previous PID-only behavior. stop no longer waits on or force-kills an unrelated process that reused the PID, and logs uses the recorded start time. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved process-identity handling can allow forced termination of an unverified process and deletion of a live instance’s authentication token when a PID is reused.
2 open issues
What changed in this PR
This PR addresses #1755 by recording process start times to distinguish detached Dev Proxy instances from unrelated processes that reuse their PIDs.
Changes:
- Adds start-time checks to instance liveness and stale-state handling, preserving PID-only behavior for legacy records.
- Applies identity checks during graceful and forced stops, and uses recorded start times for logs.
- Adds tests for serialization, identity matching, legacy compatibility, and log start times.
| File | Description |
|---|---|
| DevProxy/State/StateManager.cs | Adds process identity checks to liveness and stale-state detection. |
| DevProxy/State/ProxyInstanceState.cs | Adds the optional processStartTime field. |
| DevProxy/Commands/StopCommand.cs | Checks process identity while waiting and before forced termination. |
| DevProxy/Commands/LogsCommand.cs | Prefers the recorded process start time. |
| DevProxy/Commands/DevProxyCommand.cs | Records process start time when saving instance state. |
| DevProxy.Tests/StateManagerPidReuseTests.cs | Tests process identity handling and backward compatibility. |
Review effort: Balanced
- stop --force refuses to terminate a process whose start time can't be read when the state recorded one. - PID-keyed API tokens are only deleted when no process with that PID is running, so a new instance that reused the PID keeps its token. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
- stop --force on an instance that's already gone (exited or PID reused) restores the system proxy only when no live instance owns it. - State records are re-read before deletion and removed only if they still describe the same instance (PID + start time). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This branch has not been deployed
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.
Closes #1755
Detached instances were considered alive if any process with the recorded PID existed. Because the OS recycles PIDs, a stale state record could look alive.
Changes
ProxyInstanceStatenow storesprocessStartTime(UTC), set when the state is written.StateManager.IsInstanceAlivechecks that the process exists and that its start time matches the recorded value (1s tolerance, because Linux derives start time from boot time). A mismatch means the PID was reused, so the record is treated as stale. It's used by state loading/pruning (stop,status,logs,api,LoadAllStatesAsync,FindSystemProxyInstanceAsync) and byGetOrphanedSystemProxyStatesAsync.stop: the graceful-stop wait counts a reused PID as exited, and--forcechecks the start time beforeKill(), so it never kills an unrelated process.logsuses the recorded start time when it's available.Validation
StateManagerPidReuseTests; DevProxy.Tests passes (138/138).processStartTime. After the start time was edited in the file,statusreported the instance as not running and pruned the record, andstop --force --pidleft the live process running.