Reconnect the terminal view when somebody comes back to it #14

Merged
jaap-jan merged 2 commits from claude/terminal-reconnection-stuck-4e57ff into main 2026-08-14 14:13:29 +00:00
Owner

Going back to a terminal left alone for a while found it stuck on
"Reconnecting the terminal view…", and stuck is the right word: the banner
stayed and nothing behind it was reconnecting.

The page's whole recovery story was a setTimeout chain, and a chain is exactly
what a WebView is entitled to stop running. Chromium throttles timers in a page
nobody is looking at — down to once a minute once it has been hidden five
minutes — and a renderer that is frozen, or reclaimed and not yet reloaded,
runs none of them. So the socket drops while nobody is watching, the banner
goes up, the retry is scheduled, and the retry is then the one thing not
running.

Three defects, each of which leaves that banner up for the rest of the page's
life.

◆ NOTHING LISTENED FOR THE PAGE COMING BACK. The only thing that could clear
the banner was a timer that may never fire. terminal.js now reconnects on
visibilitychange, focus and online — the events that mean somebody is looking
again, and the ones that cannot be throttled — cancelling the pending timer and
resetting the backoff. Over a healthy socket all three do nothing, which is
what makes them safe to fire as often as clicking a window does.

◆ A HANDSHAKE THAT NEVER FINISHED WAS INVISIBLE. Every retry was scheduled by a
close or an error, so an attempt parked in CONNECTING — which is what a
suspended renderer leaves behind — scheduled nothing at all, ever. There is now
a five-second watchdog on the handshake.

◆ STALE SOCKETS SCHEDULED RETRIES, AND THAT ONE IS A LOOP RATHER THAN A STALL.
connect() never detached the old socket's handlers, and the host aborts the
displaced socket on takeover — TerminalDataPlane.UpgradeAsync, doing exactly
what it should. That close read as a fresh failure and scheduled a retry
against the socket that had just succeeded, whose own close scheduled the next:
no fixed point, reconnecting every second forever with the banner up for most
of it. Every handler now asks whether it is still the page's own attempt, and
connect() closes what it abandons.

◆ WHICH OF THE PLATFORM BEHAVIOURS ACTUALLY BIT IS NOT ESTABLISHED, and the fix
does not depend on knowing. Throttled timers, a frozen renderer and a reclaimed
one all end at the same dead timer; guessing between them would have produced a
narrower fix for one of the three.

THE TEST RUNS terminal.js ITSELF, in a fake browser, inside dotnet test.
RendererPage loads the file the shell project ships — not a transcription of its
logic into C#, which would be a copy that stays correct while the page rots —
into a Jint engine, one per test, over a harness that fakes a WebSocket and a
clock and nothing else. Jint rather than a node script because CI would run the
node one and nobody's inner loop would; the cost is that Jint is not Chromium,
so this proves the page's logic and nothing about how a WebView behaves. That
line is drawn in RendererPage's remark and picked up by two new manual checks,
1.10 for the desktop and 11.12a for the phone, which own the platform half.

Four of the nine tests fail against the page as it stood — the stale close, the
parked handshake, and the two wake-ups. Two more assert that a wake-up over a
healthy socket does nothing, and pass against either version on purpose: they
are what stops the cure being worse.

Left alone deliberately: a socket that is open and dead shows no banner at all,
because readyState still reads OPEN. That looks like a terminal that swallows
what is typed, needs a liveness probe rather than a faster retry, and is written
down at the end of 11.12a rather than quietly bundled in here.

Going back to a terminal left alone for a while found it stuck on "Reconnecting the terminal view…", and stuck is the right word: the banner stayed and nothing behind it was reconnecting. The page's whole recovery story was a setTimeout chain, and a chain is exactly what a WebView is entitled to stop running. Chromium throttles timers in a page nobody is looking at — down to once a minute once it has been hidden five minutes — and a renderer that is frozen, or reclaimed and not yet reloaded, runs none of them. So the socket drops while nobody is watching, the banner goes up, the retry is scheduled, and the retry is then the one thing not running. Three defects, each of which leaves that banner up for the rest of the page's life. ◆ NOTHING LISTENED FOR THE PAGE COMING BACK. The only thing that could clear the banner was a timer that may never fire. terminal.js now reconnects on visibilitychange, focus and online — the events that mean somebody is looking again, and the ones that cannot be throttled — cancelling the pending timer and resetting the backoff. Over a healthy socket all three do nothing, which is what makes them safe to fire as often as clicking a window does. ◆ A HANDSHAKE THAT NEVER FINISHED WAS INVISIBLE. Every retry was scheduled by a close or an error, so an attempt parked in CONNECTING — which is what a suspended renderer leaves behind — scheduled nothing at all, ever. There is now a five-second watchdog on the handshake. ◆ STALE SOCKETS SCHEDULED RETRIES, AND THAT ONE IS A LOOP RATHER THAN A STALL. connect() never detached the old socket's handlers, and the host aborts the displaced socket on takeover — TerminalDataPlane.UpgradeAsync, doing exactly what it should. That close read as a fresh failure and scheduled a retry against the socket that had just succeeded, whose own close scheduled the next: no fixed point, reconnecting every second forever with the banner up for most of it. Every handler now asks whether it is still the page's own attempt, and connect() closes what it abandons. ◆ WHICH OF THE PLATFORM BEHAVIOURS ACTUALLY BIT IS NOT ESTABLISHED, and the fix does not depend on knowing. Throttled timers, a frozen renderer and a reclaimed one all end at the same dead timer; guessing between them would have produced a narrower fix for one of the three. THE TEST RUNS terminal.js ITSELF, in a fake browser, inside dotnet test. RendererPage loads the file the shell project ships — not a transcription of its logic into C#, which would be a copy that stays correct while the page rots — into a Jint engine, one per test, over a harness that fakes a WebSocket and a clock and nothing else. Jint rather than a node script because CI would run the node one and nobody's inner loop would; the cost is that Jint is not Chromium, so this proves the page's logic and nothing about how a WebView behaves. That line is drawn in RendererPage's remark and picked up by two new manual checks, 1.10 for the desktop and 11.12a for the phone, which own the platform half. Four of the nine tests fail against the page as it stood — the stale close, the parked handshake, and the two wake-ups. Two more assert that a wake-up over a healthy socket does nothing, and pass against either version on purpose: they are what stops the cure being worse. Left alone deliberately: a socket that is open and dead shows no banner at all, because readyState still reads OPEN. That looks like a terminal that swallows what is typed, needs a liveness probe rather than a faster retry, and is written down at the end of 11.12a rather than quietly bundled in here.
jaap-jan added 1 commit 2026-08-14 13:09:55 +00:00
Reconnect the terminal view when somebody comes back to it
ci / build and test (pull_request) Failing after 2m29s
ci / desktop nightly (pull_request) Skipped
ci / api image (pull_request) Skipped
ci / android head (pull_request) Successful in 3m23s
963cb7f670
Going back to a terminal left alone for a while found it stuck on
"Reconnecting the terminal view…", and stuck is the right word: the banner
stayed and nothing behind it was reconnecting.

The page's whole recovery story was a setTimeout chain, and a chain is exactly
what a WebView is entitled to stop running. Chromium throttles timers in a page
nobody is looking at — down to once a minute once it has been hidden five
minutes — and a renderer that is frozen, or reclaimed and not yet reloaded,
runs none of them. So the socket drops while nobody is watching, the banner
goes up, the retry is scheduled, and the retry is then the one thing not
running.

Three defects, each of which leaves that banner up for the rest of the page's
life.

◆ NOTHING LISTENED FOR THE PAGE COMING BACK. The only thing that could clear
the banner was a timer that may never fire. terminal.js now reconnects on
visibilitychange, focus and online — the events that mean somebody is looking
again, and the ones that cannot be throttled — cancelling the pending timer and
resetting the backoff. Over a healthy socket all three do nothing, which is
what makes them safe to fire as often as clicking a window does.

◆ A HANDSHAKE THAT NEVER FINISHED WAS INVISIBLE. Every retry was scheduled by a
close or an error, so an attempt parked in CONNECTING — which is what a
suspended renderer leaves behind — scheduled nothing at all, ever. There is now
a five-second watchdog on the handshake.

◆ STALE SOCKETS SCHEDULED RETRIES, AND THAT ONE IS A LOOP RATHER THAN A STALL.
connect() never detached the old socket's handlers, and the host aborts the
displaced socket on takeover — TerminalDataPlane.UpgradeAsync, doing exactly
what it should. That close read as a fresh failure and scheduled a retry
against the socket that had just succeeded, whose own close scheduled the next:
no fixed point, reconnecting every second forever with the banner up for most
of it. Every handler now asks whether it is still the page's own attempt, and
connect() closes what it abandons.

◆ WHICH OF THE PLATFORM BEHAVIOURS ACTUALLY BIT IS NOT ESTABLISHED, and the fix
does not depend on knowing. Throttled timers, a frozen renderer and a reclaimed
one all end at the same dead timer; guessing between them would have produced a
narrower fix for one of the three.

THE TEST RUNS terminal.js ITSELF, in a fake browser, inside dotnet test.
RendererPage loads the file the shell project ships — not a transcription of its
logic into C#, which would be a copy that stays correct while the page rots —
into a Jint engine, one per test, over a harness that fakes a WebSocket and a
clock and nothing else. Jint rather than a node script because CI would run the
node one and nobody's inner loop would; the cost is that Jint is not Chromium,
so this proves the page's logic and nothing about how a WebView behaves. That
line is drawn in RendererPage's remark and picked up by two new manual checks,
1.10 for the desktop and 11.12a for the phone, which own the platform half.

Four of the nine tests fail against the page as it stood — the stale close, the
parked handshake, and the two wake-ups. Two more assert that a wake-up over a
healthy socket does nothing, and pass against either version on purpose: they
are what stops the cure being worse.

Left alone deliberately: a socket that is open and dead shows no banner at all,
because readyState still reads OPEN. That looks like a terminal that swallows
what is typed, needs a liveness probe rather than a faster retry, and is written
down at the end of 11.12a rather than quietly bundled in here.
jaap-jan added 1 commit 2026-08-14 13:44:09 +00:00
Stop the data plane's tests racing the socket they just connected
ci / build and test (pull_request) Successful in 2m30s
ci / android head (pull_request) Successful in 3m19s
ci / desktop nightly (pull_request) Skipped
ci / api image (pull_request) Successful in 21s
ef12e8cc99
CI went red on the run that added the renderer tests, and not on anything they
assert: TerminalDataPlaneTests.Output_ReachesTheRenderer read the output frame
where it expected the session's opening one, having lost a race that has been
in the helper since it was written.

Connected and attached are two different moments. ClientWebSocket.ConnectAsync
completes on the 101, which UpgradeAsync writes before it has a WebSocket to
attach — it builds one from the stream and swaps it in a few instructions
later, on the accept thread. SendAsync drops anything sent in between, which is
the transport's documented contract rather than a bug: there is nowhere to put
a frame for a renderer that is not there, and queueing it is the unbounded
growth the credit window exists to prevent. So a helper that returned on the
handshake and let its caller send immediately was betting on thread scheduling,
every run, on every test in the file.

It only started losing now because that assembly grew nine tests and a
JavaScript engine to run them in, which is more work in parallel with a window
measured in instructions. The race is older than the branch that exposed it.

ConnectAsync now waits for the plane's own SocketAttached, subscribed before
the connection because the event can be over before ConnectAsync returns, and
bounded so a socket that never attaches fails the helper instead of hanging the
suite in a later receive. TerminalWorkspaceTests.ConnectRendererAsync has the
same exposure through OpenSessionAsync's opening frame and now waits on
WaitForRendererAsync, with a note on why that is enough for the reattach tests
and what would stop being enough.

◆ NOTHING IN src CHANGED, AND THAT IS THE CONCLUSION RATHER THAN THE SHORTCUT.
Production waits for exactly this moment already — every path that opens a
session goes through TerminalWorkspace.WaitForRendererAsync, which resolves
from the same few lines that raise the event this helper now waits on. Only the
tests skipped the gate the application does not.

Verified by widening the window rather than by hunting the flake: a 100ms delay
inserted between the 101 and the attach, in a throwaway tree, hangs the old
helper outright — both frames dropped, the test blocked in receive — and passes
89/89 with this one. Green three times over on the CI platform besides
(Alpine, musl, Release, dotnet/sdk:10.0-alpine).
jaap-jan merged commit 2f8daacb60 into main 2026-08-14 14:13:29 +00:00
Sign in to join this conversation.
No Reviewers
No labels
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: DodoTech-Public/DodoSSH#14