Restore system proxy after a crashed instance#1756
Open
waldekmastykarz wants to merge 1 commit into
Open
Conversation
When a detached instance running as system proxy is terminated uncleanly, the OS proxy was left pointing at a dead port and 'devproxy stop --force' could not recover it: StateManager prunes state files for dead PIDs before the stop command reads them, so no instance was found and the system proxy was never disabled. Reconcile orphaned system-proxy registrations independently of finding a live instance: - Add StateManager.GetOrphanedSystemProxyStatesAsync (reads records for dead PIDs with asSystemProxy=true without pruning them first). - Add cross-platform, idempotent SystemProxyManager.Disable (Windows WinINET via ProxyServer.DisableAllSystemProxies, macOS toggle-proxy.sh off) and ReconcileOrphanedSystemProxiesAsync, guarded so the OS proxy is only disabled when no live instance still owns it. - devproxy stop / stop --pid now restore the system proxy for crashed instances; self-heal also runs at startup (detached launcher and ProxyEngine) so a stale registration doesn't linger across runs. Refs dotnet#1731. PID-reuse hardening tracked in dotnet#1755. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0b22cd4a-15a3-4a8c-8248-fe776339aa45
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a failure mode where a crashed detached Dev Proxy instance that enabled the OS system proxy could leave the machine’s proxy settings pointing at a dead port, and devproxy stop --force could not recover because stale state was pruned before stop logic could restore the proxy.
Changes:
- Add orphan detection in
StateManagerto surface staleasSystemProxyrecords without pruning them first. - Introduce
SystemProxyManagerto disable the OS proxy (best-effort, cross-platform) and reconcile orphaned system-proxy registrations. - Update
stopand startup paths to reconcile and clean up orphaned system-proxy state before liveness-pruning state loads run.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| DevProxy/State/StateManager.cs | Adds non-pruning state read helper and an API to enumerate orphaned system-proxy state records. |
| DevProxy/Proxy/SystemProxyManager.cs | New manager to disable OS proxy and reconcile orphaned system-proxy registrations safely. |
| DevProxy/Proxy/ProxyEngine.cs | Adds foreground startup self-heal by reconciling orphaned system-proxy registrations. |
| DevProxy/Program.cs | Adds detached launcher self-heal before state-loading/pruning operations. |
| DevProxy/Commands/StopCommand.cs | Reconciles orphaned system-proxy registrations as part of stop flows and gates force disable by AsSystemProxy. |
Comment on lines
+58
to
+70
| var reconciliation = await SystemProxyManager.ReconcileOrphanedSystemProxiesAsync(); | ||
| foreach (var orphan in reconciliation.Orphans) | ||
| { | ||
| var message = $"Recovered system proxy left by a crashed Dev Proxy instance (PID: {orphan.Pid})."; | ||
| if (isJsonOutput) | ||
| { | ||
| await Console.Out.WriteLineAsync(FormatJsonLogEntry("info", message)); | ||
| } | ||
| else | ||
| { | ||
| await Console.Out.WriteLineAsync(message); | ||
| } | ||
| } |
Comment on lines
+162
to
+171
| // Recover from a system-proxy registration left behind by a previous | ||
| // instance that crashed before restoring the OS proxy, so a stale | ||
| // registration doesn't linger across runs. | ||
| var reconciliation = await SystemProxyManager.ReconcileOrphanedSystemProxiesAsync(stoppingToken); | ||
| foreach (var orphan in reconciliation.Orphans) | ||
| { | ||
| _logger.LogInformation( | ||
| "Recovered system proxy left by a crashed Dev Proxy instance (PID: {Pid}).", | ||
| orphan.Pid); | ||
| } |
Comment on lines
+109
to
+116
| var startInfo = new ProcessStartInfo | ||
| { | ||
| FileName = "/bin/bash", | ||
| Arguments = $"{bashScriptPath} off", | ||
| RedirectStandardOutput = true, | ||
| UseShellExecute = false, | ||
| CreateNoWindow = true | ||
| }; |
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.
Summary
When a detached Dev Proxy instance running with
--as-system-proxy trueis terminated uncleanly (crash,kill -9, OOM, power loss), the OS proxy is left pointing at the now-dead port anddevproxy stop --forcecould not recover it — it printedDev Proxy is not running.and exited 1, leaving the machine without working network access.Root cause:
StateManagerprunes any state file whose PID is no longer alive before the stop command reads it, so a crashed instance's record is already gone by the timestop --forceruns ⇒ no instance is found ⇒SystemProxyManager.Disable()is never invoked.--forcetherefore only worked for a hung-but-alive instance, not the dead one it's meant to recover.The fix decouples "restore the OS proxy" from "find a live instance."
Fixes #1731.
Changes
SystemProxyManager.cs(new) — cross-platform, idempotentDisable()(Windows WinINET viaProxyServer.DisableAllSystemProxies(), macOStoggle-proxy.sh off, Linux no-op) plusReconcileOrphanedSystemProxiesAsync(). Reconciliation is guarded: the global OS proxy is only disabled when no live instance still owns it — otherwise only the stale record is removed.StateManager.cs— addedGetOrphanedSystemProxyStatesAsync()which reads records for dead PIDs withasSystemProxy: truewithout pruning them first, and extracted a non-pruningReadStateFromFileAsynchelper.StopCommand.cs—devproxy stopanddevproxy stop --pidnow reconcile crashed system-proxy orphans (restore the OS proxy + clean up the record) in addition to stopping live instances. Gated on Dev Proxy's ownasSystemProxyrecord, so a proxy Dev Proxy didn't set is never touched.Program.cs/ProxyEngine.cs— startup self-heal so a stale registration is recovered on the next run. The reconciliation runs at the very top of the detached launcher (before its state-loading port-conflict check, which would otherwise prune the orphan without restoring the proxy) and inProxyEnginefor foreground runs.Safety
asSystemProxy: true— it never disables a corporate/third-party proxy.Testing (macOS)
Verified with the built binary (system proxy was off, so
Disable()was a safe no-op):stoprestores it, deletes the record, exit 0Dev Proxy is not running., exit 1stop --pidon a crashed orphan → restoredRecovered system proxy left by a crashed Dev Proxy instancelogged, orphan cleaned upFull solution builds clean (0 warnings, 0 errors).
Notes