diff --git a/claude.md b/claude.md index e7659518..55140ef9 100644 --- a/claude.md +++ b/claude.md @@ -223,6 +223,10 @@ apart. quotes, braces and newlines, and the `inline` body carries an `InlinePatchFile` payload verbatim. - Compiles for every DiffEngine target, so the socket calls carry `#if` branches for the frameworks with no cancellation overloads. `ViewerProtocolTests` runs on all of them. +- `ViewerServer`'s accept loop awaits with `ConfigureAwait(false)`, the one place that matters + in a repo that otherwise leaves it off. The Windows viewer starts listening on its UI thread, + and resuming there left every connection waiting on the render loop to pump. + `AnOwnerAnswersWhileTheThreadThatStartedItIsBusy` pins it. **Native shim (`native/`), used by the Mac and Linux heads only:** - `raylib` and `imgui` are fetched by CMake (`FetchContent`), pinned by tag in diff --git a/src/DiffEngine.Tests/ViewerProtocolTests.cs b/src/DiffEngine.Tests/ViewerProtocolTests.cs index 7ebdbe6e..be99321e 100644 --- a/src/DiffEngine.Tests/ViewerProtocolTests.cs +++ b/src/DiffEngine.Tests/ViewerProtocolTests.cs @@ -639,6 +639,59 @@ public async Task AnOwnerAnswersAClient() await Wait(listening); } + /// + /// The Windows viewer starts listening on its UI thread, which carries a WinForms context by + /// then, and that thread is the render loop: it pumps between frames, and not at all while an + /// accept on it waits up to ten seconds on InlineApplier's mutex. The accept loop resumed on + /// it, so every connection went unanswered for as long as the render thread was busy - a + /// tray's listing, an attached viewer's poll, the next failing snapshot. + /// + /// A context that is never pumped is that thread at its worst, and the owner still answers. + /// + /// + [Test] + public async Task AnOwnerAnswersWhileTheThreadThatStartedItIsBusy() + { + await Assert.That(ViewerServer.TryBind(0, out var bound)).IsTrue(); + using var server = bound!; + using var cancel = new CancelSource(); + Task listening; + var previous = SynchronizationContext.Current; + // Only around the call, and with nothing awaited inside it: a continuation of this test + // posted to a context nobody pumps would never run + SynchronizationContext.SetSynchronizationContext(new UnpumpedContext()); + try + { + listening = server.Listen(_ => ViewerResponse.Success($"heard {_.Verb}"), cancel.Token); + } + finally + { + SynchronizationContext.SetSynchronizationContext(previous); + } + + var sent = ViewerClient.TrySend(new(ViewerVerb.List), out var response, server.Port, underLoad); + + await Assert.That(sent).IsTrue(); + await Assert.That(response!.Message).IsEqualTo("heard List"); + + await cancel.CancelAsync(); + await Wait(listening); + } + + /// + /// The one thread of a context whose owner is too busy to pump it, so whatever is posted to it + /// waits for good. + /// + sealed class UnpumpedContext : SynchronizationContext + { + public override void Post(SendOrPostCallback callback, object? state) + { + } + + public override void Send(SendOrPostCallback callback, object? state) => + throw new NotSupportedException("A thread that is not pumping cannot be sent to."); + } + /// /// Connections are handled concurrently, so one slow exchange does not stop the next from /// being answered. Accepting an inline snapshot legitimately takes seconds, and a client diff --git a/src/DiffEngine/Protocol/ViewerServer.cs b/src/DiffEngine/Protocol/ViewerServer.cs index 5c568708..d8bf3ca3 100644 --- a/src/DiffEngine/Protocol/ViewerServer.cs +++ b/src/DiffEngine/Protocol/ViewerServer.cs @@ -60,7 +60,12 @@ public async Task Listen(Func handle, Cancel canc TcpClient client; try { - client = await Accept(cancel); + // Off whatever thread started listening, here and in Accept. The Windows viewer + // starts on its UI thread, which has a WinForms context by then, and resuming there + // waited for the render loop to pump: every connection went unanswered for as long + // as that thread was busy, which an accept holding InlineApplier's mutex makes up + // to ten seconds. Both awaits, because the first Accept runs on the caller's thread. + client = await Accept(cancel).ConfigureAwait(false); } catch (OperationCanceledException) { @@ -111,12 +116,12 @@ internal static bool IsStop(SocketException exception, Cancel cancel) => async Task Accept(Cancel cancel) { #if NET6_0_OR_GREATER - return await listener.AcceptTcpClientAsync(cancel); + return await listener.AcceptTcpClientAsync(cancel).ConfigureAwait(false); #else // No token overload here, so cancellation arrives as the registered Stop, which faults // this await with one of the exceptions the caller already treats as "stop serving". cancel.ThrowIfCancellationRequested(); - return await listener.AcceptTcpClientAsync(); + return await listener.AcceptTcpClientAsync().ConfigureAwait(false); #endif }