Skip to content

Answer the viewer's port while its render thread is busy - #873

Merged
SimonCropp merged 1 commit into
mainfrom
viewer-listener-off-ui-thread
Sep 22, 2026
Merged

SimonCropp merged 1 commit into
mainfrom
viewer-listener-off-ui-thread

Conversation

@SimonCropp

Copy link
Copy Markdown
Member

ViewerServer.Listen awaited its accept without ConfigureAwait(false), and the Windows viewer calls it from its UI thread once the form exists, when that thread already carries a WinForms synchronization context. So every accept resumed on the UI thread, which is the render loop and only pumps between frames. While the render thread was busy - an owning viewer applying an accept waits up to ten seconds on InlineApplier's cross process mutex - no new connection was handled at all: the tray's listings, an attached viewer's polls and the next failing snapshot all waited for the render loop to come round.

Reproduced against the real viewer: with one source file's patch mutex held and Accept clicked on that entry, a listfull sent to the viewer did not answer within a second for the whole five seconds the mutex was held, and answered at once after it was let go. With this change it answers throughout.

Both awaits in the accept chain take ConfigureAwait(false). The first Accept runs on the caller's thread, so fixing Listen's await alone still parks Accept's continuation on the UI thread. Each connection's handler already ran on the pool. The Mac and Linux heads install no context and were never affected.

AnOwnerAnswersWhileTheThreadThatStartedItIsBusy starts the listener under a single threaded context that is never pumped, and asserts a client is answered. It times out on the old code, and on the new code with either await left as it was. The repo otherwise leaves ConfigureAwait off, so CLAUDE.md says why this one stays.

ViewerServer.Listen awaited its accept without ConfigureAwait(false),
and the Windows viewer calls it from its UI thread once the form
exists, when that thread already carries a WinForms synchronization
context. So every accept resumed on the UI thread, which is the render
loop and only pumps between frames. While the render thread was busy -
an owning viewer applying an accept waits up to ten seconds on
InlineApplier's cross process mutex - no new connection was handled at
all: the tray's listings, an attached viewer's polls and the next
failing snapshot all waited for the render loop to come round.

Reproduced against the real viewer: with one source file's patch mutex
held and Accept clicked on that entry, a listfull sent to the viewer
did not answer within a second for the whole five seconds the mutex was
held, and answered at once after it was let go. With this change it
answers throughout.

Both awaits in the accept chain take ConfigureAwait(false). The first
Accept runs on the caller's thread, so fixing Listen's await alone
still parks Accept's continuation on the UI thread. Each connection's
handler already ran on the pool. The Mac and Linux heads install no
context and were never affected.

AnOwnerAnswersWhileTheThreadThatStartedItIsBusy starts the listener
under a single threaded context that is never pumped, and asserts a
client is answered. It times out on the old code, and on the new code
with either await left as it was.
@SimonCropp
SimonCropp merged commit dcdf37a into main Sep 22, 2026
9 checks passed
@SimonCropp
SimonCropp deleted the viewer-listener-off-ui-thread branch September 22, 2026 00:57
@SimonCropp SimonCropp added this to the 20.4.0 milestone Sep 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant