Found during the v0.6.0 pre-release review. Low severity on its own, but it likely compounds #82.
The bug
PseudoTerminal.MonitorExitAsync (added in #63 to fix the handle race in #39) returns early on a failed DuplicateHandle — before Exited?.Invoke():
if (!DuplicateHandle(GetCurrentProcess(), _hProcess, GetCurrentProcess(),
out IntPtr waitHandle, 0, false, DUPLICATE_SAME_ACCESS))
{
Log($"DuplicateHandle failed: {Marshal.GetLastWin32Error()}");
return; // <-- Exited never fires
}
try { ... }
finally { CloseHandle(waitHandle); }
Exited?.Invoke();
Every other path fires it exactly once. This one silently doesn't.
Why it matters
MainWindow.DisposeAndWaitForExitAsync subscribes to Exited and waits on a TaskCompletionSource:
await Task.WhenAny(tcs.Task, Task.Delay(timeoutMs)); // timeoutMs: 10000
If Exited never fires, that session waits out its full 10-second timeout. The disposal loop is sequential, so with N affected sessions it's N × 10s added to shutdown.
Bounded rather than hanging, and DuplicateHandle on a process handle you already own effectively never fails — which is why this is low severity rather than urgent.
Fix
Move the invoke into a finally so every path fires it exactly once:
try
{
if (!DuplicateHandle(...)) { Log(...); return; }
try
{
await Task.Run(() => WaitForSingleObject(waitHandle, 0xFFFFFFFF));
if (GetExitCodeProcess(waitHandle, out uint code)) ExitCode = unchecked((int)code);
}
finally { CloseHandle(waitHandle); }
}
finally { Exited?.Invoke(); }
Worth a unit test via the IPseudoTerminal seam asserting Exited fires on the failure path too.
Relationship to #82
#82 is about startup/shutdown scaling badly with live session count, attributed mostly to fixed sleeps. This is a second, independent contributor to the same symptom — a conditional 10s per session rather than a fixed delay. Worth ruling in or out while working #82, since with 40 sessions the two are hard to tell apart from the outside.
Found during the v0.6.0 pre-release review. Low severity on its own, but it likely compounds #82.
The bug
PseudoTerminal.MonitorExitAsync(added in #63 to fix the handle race in #39) returns early on a failedDuplicateHandle— beforeExited?.Invoke():Every other path fires it exactly once. This one silently doesn't.
Why it matters
MainWindow.DisposeAndWaitForExitAsyncsubscribes toExitedand waits on aTaskCompletionSource:If
Exitednever fires, that session waits out its full 10-second timeout. The disposal loop is sequential, so with N affected sessions it's N × 10s added to shutdown.Bounded rather than hanging, and
DuplicateHandleon a process handle you already own effectively never fails — which is why this is low severity rather than urgent.Fix
Move the invoke into a
finallyso every path fires it exactly once:Worth a unit test via the
IPseudoTerminalseam assertingExitedfires on the failure path too.Relationship to #82
#82 is about startup/shutdown scaling badly with live session count, attributed mostly to fixed sleeps. This is a second, independent contributor to the same symptom — a conditional 10s per session rather than a fixed delay. Worth ruling in or out while working #82, since with 40 sessions the two are hard to tell apart from the outside.