fix(ui): bound the ConPTY launch-gate wait so a hung launch can't freeze other panes
StartAsync awaited the static launch-gate semaphore with no timeout, so one hung launch (slow disk, AV scanning claude.exe, a Porta.Pty/ConPTY hiccup) blocked every subsequent pane open behind it indefinitely, including Retry. Extracted the wait into WaitForLaunchGateAsync with a 30s timeout that throws TimeoutException before the try/finally (never releasing a gate it didn't acquire); the exception flows through StartCoreAsync's existing catch into StartError, so the pane shows an error + Retry instead of hanging. StartError renders ex.Message directly (no locale key involved), so no locale changes were needed. PtyTerminalSession.StartAsync itself needs a real TerminalControl and stays impractical to unit-test directly, so tests target the extracted WaitForLaunchGateAsync helper against a plain SemaphoreSlim: timeout throws, timeout never acquires the gate, and success still acquires it.
This commit is contained in:
@@ -25,7 +25,21 @@ public sealed class PtyTerminalSession : IDisposable
|
|||||||
// inherits the other's env (e.g. CLAUDEDO_PLANNING_TOKEN, breaking that session's own MCP
|
// inherits the other's env (e.g. CLAUDEDO_PLANNING_TOKEN, breaking that session's own MCP
|
||||||
// auth). Process-wide env leakage AFTER a launch has forked remains a documented limitation
|
// auth). Process-wide env leakage AFTER a launch has forked remains a documented limitation
|
||||||
// — Porta.Pty has no per-launch env seam, so the vars stay set on the whole UI process.
|
// — Porta.Pty has no per-launch env seam, so the vars stay set on the whole UI process.
|
||||||
|
// The wait is bounded (see WaitForLaunchGateAsync) — a hung launch (slow disk, AV scanning
|
||||||
|
// claude.exe, a Porta.Pty/ConPTY hiccup) must not freeze every other pane open behind it.
|
||||||
private static readonly SemaphoreSlim s_launchGate = new(1, 1);
|
private static readonly SemaphoreSlim s_launchGate = new(1, 1);
|
||||||
|
private static readonly TimeSpan s_launchGateTimeout = TimeSpan.FromSeconds(30);
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Waits on <paramref name="gate"/> for at most <paramref name="timeout"/>, throwing
|
||||||
|
/// <see cref="TimeoutException"/> instead of blocking forever. Never acquires the gate on
|
||||||
|
/// timeout, so callers must not release it in that case.
|
||||||
|
/// </summary>
|
||||||
|
internal static async Task WaitForLaunchGateAsync(SemaphoreSlim gate, TimeSpan timeout, CancellationToken ct)
|
||||||
|
{
|
||||||
|
if (!await gate.WaitAsync(timeout, ct))
|
||||||
|
throw new TimeoutException("Another terminal launch is still starting up. Please retry in a moment.");
|
||||||
|
}
|
||||||
|
|
||||||
private TerminalControl? _control;
|
private TerminalControl? _control;
|
||||||
private bool _disposed;
|
private bool _disposed;
|
||||||
@@ -50,7 +64,7 @@ public sealed class PtyTerminalSession : IDisposable
|
|||||||
control.Args = new List<string>(descriptor.Args);
|
control.Args = new List<string>(descriptor.Args);
|
||||||
control.StartingDirectory = descriptor.Cwd;
|
control.StartingDirectory = descriptor.Cwd;
|
||||||
|
|
||||||
await s_launchGate.WaitAsync(ct);
|
await WaitForLaunchGateAsync(s_launchGate, s_launchGateTimeout, ct);
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
foreach (var (key, value) in descriptor.Env)
|
foreach (var (key, value) in descriptor.Env)
|
||||||
|
|||||||
@@ -0,0 +1,38 @@
|
|||||||
|
using System.Threading;
|
||||||
|
using ClaudeDo.Ui.Services;
|
||||||
|
using Xunit;
|
||||||
|
|
||||||
|
namespace ClaudeDo.Ui.Tests.Services;
|
||||||
|
|
||||||
|
public class PtyTerminalSessionTests
|
||||||
|
{
|
||||||
|
[Fact]
|
||||||
|
public async Task WaitForLaunchGateAsync_ThrowsTimeoutException_WhenGateStaysHeld()
|
||||||
|
{
|
||||||
|
using var gate = new SemaphoreSlim(0, 1); // never released — simulates a hung launch
|
||||||
|
|
||||||
|
await Assert.ThrowsAsync<TimeoutException>(() =>
|
||||||
|
PtyTerminalSession.WaitForLaunchGateAsync(gate, TimeSpan.FromMilliseconds(50), CancellationToken.None));
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task WaitForLaunchGateAsync_DoesNotReleaseGate_ItNeverAcquired()
|
||||||
|
{
|
||||||
|
using var gate = new SemaphoreSlim(0, 1);
|
||||||
|
|
||||||
|
await Assert.ThrowsAsync<TimeoutException>(() =>
|
||||||
|
PtyTerminalSession.WaitForLaunchGateAsync(gate, TimeSpan.FromMilliseconds(50), CancellationToken.None));
|
||||||
|
|
||||||
|
Assert.Equal(0, gate.CurrentCount); // still held; a bad fix would Release() what it never acquired
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task WaitForLaunchGateAsync_AcquiresGate_WhenAvailable()
|
||||||
|
{
|
||||||
|
using var gate = new SemaphoreSlim(1, 1);
|
||||||
|
|
||||||
|
await PtyTerminalSession.WaitForLaunchGateAsync(gate, TimeSpan.FromSeconds(5), CancellationToken.None);
|
||||||
|
|
||||||
|
Assert.Equal(0, gate.CurrentCount); // acquired
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user