Skip to content

Commit 66c7800

Browse files
PureWeenCopilot
PureWeen
and
Copilot
committed
fix: prevent Process.HasExited race causing UnobservedTaskException
The crash log shows InvalidOperationException ('No process is associated with this object') when Process.HasExited is accessed on a disposed process. Fire-and-forget tasks monitoring stdout/stderr in DevTunnelService accessed the _hostProcess field directly, which could be nulled/disposed by Stop() or TryHostTunnelAsync() concurrently. The same pattern existed in CodespaceService.TunnelHandle and ServerManager.StopServer. Changes: - Add ProcessHelper utility with SafeHasExited(), SafeKill(), and SafeKillAndDispose() that never throw on disposed/invalid processes - DevTunnelService: capture process in local variable before passing to fire-and-forget tasks; use ProcessHelper.SafeHasExited() in loops - CodespaceService.TunnelHandle: add _disposed flag so IsAlive returns false after DisposeAsync(); use SafeHasExited for process checks - ServerManager.StopServer: use SafeKillAndDispose for clean teardown - Add 11 unit tests covering null, disposed, exited, and running process states plus a concurrent-dispose regression test Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 010cbbb commit 66c7800

6 files changed

Lines changed: 332 additions & 19 deletions

File tree

‎PolyPilot.Tests/PolyPilot.Tests.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@
8989
<Compile Include="../PolyPilot/Services/PluginLoader.cs" Link="Shared/PluginLoader.cs" />
9090
<Compile Include="../PolyPilot/Services/PluginFileLogger.cs" Link="Shared/PluginFileLogger.cs" />
9191
<Compile Include="../PolyPilot/Services/ProviderHostContext.cs" Link="Shared/ProviderHostContext.cs" />
92+
<Compile Include="../PolyPilot/Services/ProcessHelper.cs" Link="Shared/ProcessHelper.cs" />
9293
<Compile Include="../PolyPilot/Models/PluginSettings.cs" Link="Shared/PluginSettings.cs" />
9394
</ItemGroup>
9495

‎PolyPilot.Tests/ProcessHelperTests.cs‎

Lines changed: 242 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,242 @@
1+
using System.Diagnostics;
2+
using PolyPilot.Services;
3+
4+
namespace PolyPilot.Tests;
5+
6+
/// <summary>
7+
/// Tests for ProcessHelper — safe wrappers for Process lifecycle operations
8+
/// that prevent InvalidOperationException / UnobservedTaskException crashes
9+
/// when a process is disposed while background tasks are still monitoring it.
10+
/// </summary>
11+
public class ProcessHelperTests
12+
{
13+
// ===== SafeHasExited =====
14+
15+
[Fact]
16+
public void SafeHasExited_NullProcess_ReturnsTrue()
17+
{
18+
Assert.True(ProcessHelper.SafeHasExited(null));
19+
}
20+
21+
[Fact]
22+
public void SafeHasExited_DisposedProcess_ReturnsTrue()
23+
{
24+
// Start a short-lived process and dispose it immediately
25+
var psi = new ProcessStartInfo
26+
{
27+
FileName = OperatingSystem.IsWindows() ? "cmd.exe" : "/bin/sh",
28+
Arguments = OperatingSystem.IsWindows() ? "/c echo test" : "-c \"echo test\"",
29+
UseShellExecute = false,
30+
RedirectStandardOutput = true,
31+
CreateNoWindow = true
32+
};
33+
var process = Process.Start(psi)!;
34+
process.WaitForExit(5000);
35+
process.Dispose();
36+
37+
// After disposal, HasExited would throw InvalidOperationException.
38+
// SafeHasExited must return true instead.
39+
Assert.True(ProcessHelper.SafeHasExited(process));
40+
}
41+
42+
[Fact]
43+
public void SafeHasExited_ExitedProcess_ReturnsTrue()
44+
{
45+
var psi = new ProcessStartInfo
46+
{
47+
FileName = OperatingSystem.IsWindows() ? "cmd.exe" : "/bin/sh",
48+
Arguments = OperatingSystem.IsWindows() ? "/c echo done" : "-c \"echo done\"",
49+
UseShellExecute = false,
50+
RedirectStandardOutput = true,
51+
CreateNoWindow = true
52+
};
53+
var process = Process.Start(psi)!;
54+
process.WaitForExit(5000);
55+
56+
Assert.True(ProcessHelper.SafeHasExited(process));
57+
process.Dispose();
58+
}
59+
60+
[Fact]
61+
public void SafeHasExited_RunningProcess_ReturnsFalse()
62+
{
63+
// Start a long-running process
64+
var psi = new ProcessStartInfo
65+
{
66+
FileName = OperatingSystem.IsWindows() ? "cmd.exe" : "/bin/sh",
67+
Arguments = OperatingSystem.IsWindows() ? "/c ping -n 30 127.0.0.1 > nul" : "-c \"sleep 30\"",
68+
UseShellExecute = false,
69+
CreateNoWindow = true
70+
};
71+
var process = Process.Start(psi)!;
72+
try
73+
{
74+
Assert.False(ProcessHelper.SafeHasExited(process));
75+
}
76+
finally
77+
{
78+
try { process.Kill(true); } catch { }
79+
process.Dispose();
80+
}
81+
}
82+
83+
// ===== SafeKill =====
84+
85+
[Fact]
86+
public void SafeKill_NullProcess_DoesNotThrow()
87+
{
88+
ProcessHelper.SafeKill(null);
89+
}
90+
91+
[Fact]
92+
public void SafeKill_DisposedProcess_DoesNotThrow()
93+
{
94+
var psi = new ProcessStartInfo
95+
{
96+
FileName = OperatingSystem.IsWindows() ? "cmd.exe" : "/bin/sh",
97+
Arguments = OperatingSystem.IsWindows() ? "/c echo test" : "-c \"echo test\"",
98+
UseShellExecute = false,
99+
CreateNoWindow = true
100+
};
101+
var process = Process.Start(psi)!;
102+
process.WaitForExit(5000);
103+
process.Dispose();
104+
105+
// Must not throw
106+
ProcessHelper.SafeKill(process);
107+
}
108+
109+
[Fact]
110+
public void SafeKill_RunningProcess_KillsIt()
111+
{
112+
var psi = new ProcessStartInfo
113+
{
114+
FileName = OperatingSystem.IsWindows() ? "cmd.exe" : "/bin/sh",
115+
Arguments = OperatingSystem.IsWindows() ? "/c ping -n 30 127.0.0.1 > nul" : "-c \"sleep 30\"",
116+
UseShellExecute = false,
117+
CreateNoWindow = true
118+
};
119+
var process = Process.Start(psi)!;
120+
121+
ProcessHelper.SafeKill(process);
122+
process.WaitForExit(5000);
123+
Assert.True(process.HasExited);
124+
process.Dispose();
125+
}
126+
127+
// ===== SafeKillAndDispose =====
128+
129+
[Fact]
130+
public void SafeKillAndDispose_NullProcess_DoesNotThrow()
131+
{
132+
ProcessHelper.SafeKillAndDispose(null);
133+
}
134+
135+
[Fact]
136+
public void SafeKillAndDispose_AlreadyDisposed_DoesNotThrow()
137+
{
138+
var psi = new ProcessStartInfo
139+
{
140+
FileName = OperatingSystem.IsWindows() ? "cmd.exe" : "/bin/sh",
141+
Arguments = OperatingSystem.IsWindows() ? "/c echo test" : "-c \"echo test\"",
142+
UseShellExecute = false,
143+
CreateNoWindow = true
144+
};
145+
var process = Process.Start(psi)!;
146+
process.WaitForExit(5000);
147+
process.Dispose();
148+
149+
// Calling SafeKillAndDispose on already-disposed process must not throw
150+
ProcessHelper.SafeKillAndDispose(process);
151+
}
152+
153+
[Fact]
154+
public void SafeKillAndDispose_RunningProcess_KillsAndDisposes()
155+
{
156+
var psi = new ProcessStartInfo
157+
{
158+
FileName = OperatingSystem.IsWindows() ? "cmd.exe" : "/bin/sh",
159+
Arguments = OperatingSystem.IsWindows() ? "/c ping -n 30 127.0.0.1 > nul" : "-c \"sleep 30\"",
160+
UseShellExecute = false,
161+
CreateNoWindow = true
162+
};
163+
var process = Process.Start(psi)!;
164+
var pid = process.Id;
165+
166+
ProcessHelper.SafeKillAndDispose(process);
167+
168+
// Verify the process is no longer running
169+
try
170+
{
171+
var check = Process.GetProcessById(pid);
172+
// Process might still be there for a moment — give it time
173+
check.WaitForExit(2000);
174+
}
175+
catch (ArgumentException)
176+
{
177+
// Process already gone — expected
178+
}
179+
}
180+
181+
// ===== Race condition regression test =====
182+
183+
[Fact]
184+
public void SafeHasExited_ConcurrentDispose_NoUnobservedTaskException()
185+
{
186+
// Regression test: simulates the race condition where a background task
187+
// checks HasExited while another thread disposes the process.
188+
using var unobservedSignal = new ManualResetEventSlim(false);
189+
Exception? unobservedException = null;
190+
EventHandler<UnobservedTaskExceptionEventArgs> handler = (sender, args) =>
191+
{
192+
if (args.Exception?.InnerException is InvalidOperationException)
193+
{
194+
unobservedException = args.Exception;
195+
unobservedSignal.Set();
196+
}
197+
};
198+
199+
TaskScheduler.UnobservedTaskException += handler;
200+
try
201+
{
202+
for (int i = 0; i < 5; i++)
203+
{
204+
var psi = new ProcessStartInfo
205+
{
206+
FileName = OperatingSystem.IsWindows() ? "cmd.exe" : "/bin/sh",
207+
Arguments = OperatingSystem.IsWindows() ? "/c ping -n 10 127.0.0.1 > nul" : "-c \"sleep 10\"",
208+
UseShellExecute = false,
209+
CreateNoWindow = true
210+
};
211+
var process = Process.Start(psi)!;
212+
213+
// Background task monitoring HasExited (like DevTunnel's fire-and-forget tasks)
214+
_ = Task.Run(() =>
215+
{
216+
for (int j = 0; j < 50; j++)
217+
{
218+
if (ProcessHelper.SafeHasExited(process))
219+
break;
220+
Thread.Sleep(10);
221+
}
222+
});
223+
224+
// Simulate concurrent disposal (like Stop() being called)
225+
Thread.Sleep(50);
226+
ProcessHelper.SafeKillAndDispose(process);
227+
}
228+
229+
// Force GC to surface any unobserved task exceptions
230+
GC.Collect();
231+
GC.WaitForPendingFinalizers();
232+
GC.Collect();
233+
234+
unobservedSignal.Wait(TimeSpan.FromMilliseconds(500));
235+
Assert.Null(unobservedException);
236+
}
237+
finally
238+
{
239+
TaskScheduler.UnobservedTaskException -= handler;
240+
}
241+
}
242+
}

‎PolyPilot/Services/CodespaceService.cs‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ public sealed class TunnelHandle : IAsyncDisposable
2020
public int LocalPort { get; }
2121
public bool IsSshTunnel { get; }
2222
private readonly Process _process;
23+
private volatile bool _disposed;
2324

2425
internal TunnelHandle(int localPort, Process process, bool isSshTunnel = false)
2526
{
@@ -28,18 +29,19 @@ internal TunnelHandle(int localPort, Process process, bool isSshTunnel = false)
2829
IsSshTunnel = isSshTunnel;
2930
}
3031

31-
public bool IsAlive => !_process.HasExited;
32+
public bool IsAlive => !_disposed && !ProcessHelper.SafeHasExited(_process);
3233

3334
public async ValueTask DisposeAsync()
3435
{
36+
_disposed = true;
3537
try
3638
{
37-
if (!_process.HasExited)
39+
if (!ProcessHelper.SafeHasExited(_process))
3840
_process.Kill(entireProcessTree: true);
3941
await _process.WaitForExitAsync(CancellationToken.None).WaitAsync(TimeSpan.FromSeconds(3));
4042
}
4143
catch { }
42-
_process.Dispose();
44+
try { _process.Dispose(); } catch { }
4345
}
4446
}
4547
/// <summary>

‎PolyPilot/Services/DevTunnelService.cs‎

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -276,11 +276,7 @@ public async Task<bool> HostAsync(int copilotPort)
276276
private async Task<bool> TryHostTunnelAsync(ConnectionSettings settings)
277277
{
278278
// Kill any existing host process from a previous attempt
279-
if (_hostProcess != null && !_hostProcess.HasExited)
280-
{
281-
try { _hostProcess.Kill(entireProcessTree: true); } catch { }
282-
}
283-
_hostProcess?.Dispose();
279+
ProcessHelper.SafeKillAndDispose(_hostProcess);
284280
_hostProcess = null;
285281

286282
var hostArgs = _tunnelId != null
@@ -304,16 +300,19 @@ private async Task<bool> TryHostTunnelAsync(ConnectionSettings settings)
304300
return false;
305301
}
306302

303+
// Capture in local variable — fire-and-forget tasks must not access _hostProcess
304+
// field, which can be nulled/disposed by Stop() or a subsequent TryHostTunnelAsync().
305+
var process = _hostProcess;
307306
var urlFound = new TaskCompletionSource<bool>();
308307
var lastErrorLine = "";
309308

310309
_ = Task.Run(async () =>
311310
{
312311
try
313312
{
314-
while (!_hostProcess.HasExited)
313+
while (!ProcessHelper.SafeHasExited(process))
315314
{
316-
var line = await _hostProcess.StandardOutput.ReadLineAsync();
315+
var line = await process.StandardOutput.ReadLineAsync();
317316
if (line == null) break;
318317
Console.WriteLine($"[DevTunnel] {line}");
319318
if (!string.IsNullOrWhiteSpace(line))
@@ -328,9 +327,9 @@ private async Task<bool> TryHostTunnelAsync(ConnectionSettings settings)
328327
{
329328
try
330329
{
331-
while (!_hostProcess.HasExited)
330+
while (!ProcessHelper.SafeHasExited(process))
332331
{
333-
var line = await _hostProcess.StandardError.ReadLineAsync();
332+
var line = await process.StandardError.ReadLineAsync();
334333
if (line == null) break;
335334
Console.WriteLine($"[DevTunnel ERR] {line}");
336335
if (!string.IsNullOrWhiteSpace(line))
@@ -454,12 +453,9 @@ public void Stop()
454453
SetState(TunnelState.Stopping);
455454
try
456455
{
457-
if (_hostProcess != null && !_hostProcess.HasExited)
458-
{
459-
_hostProcess.Kill(entireProcessTree: true);
456+
if (!ProcessHelper.SafeHasExited(_hostProcess))
460457
Console.WriteLine("[DevTunnel] Host process killed");
461-
}
462-
_hostProcess?.Dispose();
458+
ProcessHelper.SafeKillAndDispose(_hostProcess);
463459
}
464460
catch (Exception ex)
465461
{

0 commit comments

Comments
 (0)