Public Access
ad1a1291591963b409fbecd4542a88d6616ce9ad
3
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
9ead42171c |
Stop a flooding remote growing the process, and let an ended session go
The terminal's memory was bounded by an argument that is not true of the SSH library underneath it, and the whole of CreditWindow's remark was that argument: the pump stops reading when the renderer falls behind, SSH's own receive window closes, the remote sshd blocks on write, and the backpressure reaches the process producing the output. The middle step does not happen. Renci.SshNet's Channels.Channel.OnData calls its window adjustment before it raises DataReceived and re-opens the channel window as soon as it drops below one packet, and ShellStream's own handler copies the bytes into a System.Net.ArrayBuffer without blocking and without a ceiling — a buffer that doubles on demand and is not given back until the stream is disposed. So a reader that stopped reading slowed the remote down by nothing at all. It only chose whose buffer the flood landed in, and it chose the one nothing bounds. Measured against the container this repository already runs sshd in: a reader that reads nothing while `yes` runs leaves ShellStream holding 8.9 MB after one second, 26.8 MB after three and 44.3 MB after five, its capacity stepping 16 → 32 → 64 MB. That is the report of "a massive memory leak on Windows"; it is not a Windows fault, it is simply where long sessions with heavy output live. The fix is to read continuously, which is what keeps SSH.NET's buffer empty, and to put the bound where it can actually be enforced. OutputBacklog is that bound: a byte-capped queue of unsent output, 4 MiB by default, which drops its oldest bytes when full and counts what it discarded. Oldest rather than newest, because the tail of a flood is the prompt somebody is waiting for. The two bounds are now separate and each is enforced where it means something. The backlog bounds what is waiting to be sent. The credit window goes on bounding what has been sent and not yet drawn — the queue inside the WebView that nothing on this side can measure — and is reserved against in the flush loop rather than the read loop. Reading is part of neither, because reading was never what the remote was waiting on. Dropping is announced rather than silent. A terminal quietly showing a different transcript from the one the remote sent is worse than one that says it could not keep up, because the second can be believed. The line goes out under its own opcode, TerminalServerOpcode.OutputDropped, rather than spliced into the output stream: what this application says about the transcript is not part of the transcript, and keeping them apart is also what makes the arithmetic hold — every byte read is either in an Output payload or counted in BytesDropped. It opens with an SGR reset, because the cut lands at an arbitrary offset and whatever escape sequence the remote was in the middle of has been left half-said. Frames are now capped at 64 KiB and taken straight into the frame buffer. The cap is the large-object-heap threshold rather than a guess — a quarter-megabyte array allocated sixty times a second is collected only by a generation 2 pass — and the direct take removes an allocation and a copy of every byte the terminal shows. The second half is the one that made the first half's cost persist. Nothing gave a session back when the remote hung up: TerminalWorkspace kept the entry, and with it an undisposed ISshConnection — an authenticated socket, its cipher state, and a keep-alive putting a packet on the wire every thirty seconds — plus the pump and SSH.NET's read buffer at whatever size the session's busiest moment had grown it to, for as long as the tab stayed open. A tab somebody typed `exit` into an hour ago was indistinguishable from a live one in everything except what it drew. ReleaseAsync disposes both, from AnnounceEndedAsync and after the announcement: the tab is told first, because tearing down an SSH connection in front of that would be latency on the one thing the user is watching for. The dictionary entry stays, because LiveSessionCount, IsSessionLive and the tab's own ENDED and RECONNECT all read it. SendInputAsync therefore asks about liveness rather than presence, since the shell channel is gone the moment the remote is. Two faults introduced on the way here and fixed, both worth naming because neither would have been visible from the outside: The read loop left through a condition on its cancellation token, and RunAsync reads a read loop that returns as the remote having closed the channel. A tab the user closed would have reported itself as a shell that exited on its own. It throws instead. With the reader finished and a renderer that had stopped acknowledging, the flush loop span: output to send, no credit to send it with, and a wait for more output that returned at once because there was some. RunAsync never completed, so the tab was never told and the connection was never released — a session invisible and permanent. The loop now leaves for the drain as soon as the backlog is complete, and RunAsync resets the credit window alongside completing it so a flusher already parked on an acknowledgement that will never come is released. Four tests asserted the old contract and now assert its reverse, which is the point rather than a relaxation — WhenCreditRunsOut_ThePumpStopsReading is now WhenCreditRunsOut_ThePumpKeepsReadingAndTheBacklogHoldsTheLine, and has to prove both halves at once. The throughput harness gained a counting transport, because a consumer that copies every frame into a 32 MB list was the slowest thing in the pipeline and a harness whose consumer cannot keep up measures itself. The fake shell gained a pace, so "a renderer that keeps up loses nothing" can be asked of a producer a network could actually deliver rather than of one running at memory bandwidth. The test that would have caught this in the first place is ARunawayRemote_KeepsBeingReadAndStaysInsideTheBacklog, and it needs the real SSH.NET and the real sshd to say anything: a fake shell hands over bytes when it is asked, and a real channel hands them over when they arrive. Under the old code its BytesRead would have stopped at exactly WindowBytes. 121 terminal tests pass over three consecutive runs with no flakes, 105 against the container, and the app, session, layout and remaining suites are unchanged and green. Manual checks 1.10a and 1.10b cover the two symptoms a person can see: a working set that plateaus under `yes`, and a connection that goes when the shell does rather than when the tab does. |
||
|
|
e2733e5cbc |
Notice when a shell ends, say which way it ended, and offer the way back
A terminal whose machine rebooted stayed green for ever. Not "went stale and
recovered late" — for ever: the tab claimed CONNECTED, the elapsed timer went
on counting, and the only thing that ever corrected it was closing the
application. That is the defect this fixes, and it turned out to be two
separate failures wearing one symptom.
◆ THE HANG IS IN SSH.NET, AND KEEP-ALIVE ALONE WOULD NOT HAVE FIXED IT.
ShellStream ends a blocked read from exactly two places: its channel being
closed, and the session raising Disconnected. A shell that exits gives the
first. But Session raises Disconnected from one method only — OnDisconnectReceived,
for a server that politely says goodbye — and a socket that simply fails goes
down a different path entirely: RaiseError fires the error event, sets the
disconnecting flag and disposes the socket, without ever raising Disconnected.
So the stream was never disposed, its Monitor.Wait was never pulsed, and the
read blocked until the process ended. The transport was gone, the client knew
the transport was gone, and nothing woke the one thread that was waiting.
That is why the first attempt at a test for this sat at thirty seconds and
failed. Both halves are needed and neither is sufficient:
- Keep-alive makes the failure HAPPEN. An idle SSH session is a socket
nobody writes to, and TCP has no way to discover that the far end has gone
without writing to it — nothing was asked, so nothing could fail. Thirty
seconds, matching OpenSSH's own ServerAliveInterval examples. It is the
write that matters and not a reply: SSH_MSG_IGNORE need not be answered,
so this is traffic rather than a heartbeat with a deadline, and there is
no timeout invented here to be wrong about a slow link.
- Disposing the stream from the error handler makes it ARRIVE. Queued to the
thread pool and not done inline, which is not tidiness: this runs on
SSH.NET's message-listener thread, and closing a channel waits for the
server's close — processed by that same thread — so an inline dispose is
the listener thread waiting on itself.
SFTP gets the keep-alive too. Its operations are request/response and bounded
by ConnectionInfo.Timeout already, so it needs no equivalent wake-up.
◆ exit AND reboot ARE TOLD APART BY A LATCH, BECAUSE THE OBVIOUS WAY IS A RACE.
Wait for the read to end and then ask whether the transport is still up, and
against a real server the answer is wrong about half the time: this was the
only channel on the connection, so sshd hangs up a millisecond after closing
it. By the time anything could ask, the transport is gone either way — and a
poll would report every ordinary `exit` as a dropped connection.
So ISshShellSession.TransportFailure is set at the moment SSH.NET raises the
failure, which its own ordering puts strictly before the disconnect that wakes
the read. A read that has just returned zero consults a settled value: set if
the transport failed, still null if the remote merely closed the channel. The
pump turns that into a TerminalSessionOutcome — RemoteClosed, ConnectionLost,
ClosedLocally — carried on SessionEnded, which used to hand over a session id
and nothing else. A subscriber cannot work this out afterwards; by the time it
hears, there is no transport left to interrogate.
Deliberately no fourth value for "the remote rebooted". Nothing on the wire
distinguishes a reboot from a cable being pulled, and a name this client
cannot check would be a guess dressed as a fact.
◆ WHAT IS DRAWN, AND WHY IT IS NOT DRAWN OVER THE PANE.
The status bar says DISCONNECTED or SHELL EXITED where it used to say NOT
CONNECTED for both — a word that is honest about a tab still dialling and
useless about one whose shell has stopped. Red for the first and grey for the
second: one is a machine that stopped answering and the other is work
finished, and crying wolf on the ordinary end of every session is its own bug.
The tab strip's dot gains the same fourth state, on both heads.
Under the terminal, a strip carrying the reason and two buttons. It takes its
own Auto grid row rather than overlaying the renderer, and that is forced
rather than chosen: the WebView is a native child view composited above
everything Avalonia draws, so the alternative is collapsing the terminal — and
the scrollback is precisely what the user wants to read, because the last
thing the remote said is usually why it went. terminal.js already writes the
same sentence into the pane; this is the copy that does not scroll away.
RECONNECT reuses the tab in place, which is what the word means to somebody
who pressed it: the terminal they were working in comes back, where it was.
It dials through the vault's ordinary connect path, so every refusal — a key
that has left the keychain, an unknown host key, a group default that has
changed since — is decided in the one place that already decides them; what
this adds is which tab the attempt belongs to. The old session is closed as
part of it, because a dead shell still holds an authenticated connection this
process has stopped reading, and its pane still holds an xterm, a scrollback
and a WebGL context, of which a browser hands out about sixteen.
Two refusals it makes rather than pretending: a host reached by typing an
address has no keychain entry to redial, and one that wants a password typed
goes to the keychain with the sheet raised rather than being dialled with an
empty box. Both say so on the strip where the button was pressed, not only in
the vault's status on a screen that surface is not.
TRY AGAIN appears on a refused connection for the same reason, and it is worth
as much: the commonest refusal is a host that was not up yet.
◆ WHAT IS ACTUALLY VERIFIED.
Two tests against a real sshd in a container. `exit` is reported as the remote
closing the session — the one a polling implementation fails, and a fake shell
cannot show it because a fake has no server behind it deciding to hang up. And
a live shell losing its transport is reported as a lost connection, produced
by dialling through a plain TCP relay the test owns and then cutting it; the
container is shared by the collection, so killing sshd was never an option.
The first draft of that second test used LoopbackProxyTests' SOCKS forward and
timed out: disconnecting the carrier of a ForwardedPortDynamic does not close
the loopback socket it accepted. Recorded in the test, because it is a
property of SSH.NET's forwarder rather than of anything here, and it is not
what a dying machine looks like.
What no test reproduces is the waiting. A relay that closes sends a FIN, so
the socket reports itself at once; a machine switched off sends nothing, and
finding that out is the keep-alive's business and the OS's retransmit budget's
— neither of which a test can hurry along without a clock this code does not
own. A reboot is prompt in practice because the machine comes back and RSTs.
Beside those: three pump outcomes, three workspace announcements, and seven
shell-flow tests covering the drop, the clean exit, reuse of the tab, retry
after a refusal, and both ways a reconnect can honestly come to nothing.
Also fixed in passing: SshNetShellSession.IsOpen read ShellStream.CanRead,
which is documented as always true — the stream stays readable after disposal
so buffered output can be drained — so it had answered "yes" for every shell
that ever existed. It reads CanWrite now. And MainWindowViewModel's session-
ended handler moved from Dispatcher.UIThread.Post to the injected post
delegate, which is what makes any of the shell half testable at all; the
remark listing the three unreachable posts is one shorter.
|
||
|
|
eb354bcdd9 |
Add the SSH session layer and the terminal data plane
The throughput harness the plan requires before any UI, plus the SSH plumbing under it. 94 new tests, no WebView involved. Credit-based flow control is what makes `yes` survivable. A terminal renders at 60 Hz at best while a remote produces output as fast as the network allows, and the difference has to accumulate somewhere or be refused. Credit is reserved *before* reading, never after: because the pump cannot read more than the renderer has room for, the coalescing buffer is bounded by the window rather than by how fast the remote can talk. When credit runs out the pump stops reading, SSH's own receive window closes, and the remote sshd blocks -- backpressure to the source with no custom protocol. Verified by falsification, not just by passing: with the credit gate removed three tests fail, including the throughput harness's bounded-memory assertion. Acknowledgements are clamped because they cross into JavaScript, where a buggy or hostile page could otherwise claim to have rendered a gigabyte and talk the host into an unbounded read. Host key trust is enforced by *failing* the connection rather than prompting inside the handshake. SSH.NET raises verification synchronously, so consulting the user there would block the handshake on a UI round trip and deadlock the first time the prompt needed the UI thread. Unknown host and changed key become distinct exceptions the caller resolves asynchronously. A mismatch has no retry path at all: a dialog offering to continue is how users are trained to click through the one warning that actually indicates interception. A legitimately rebuilt server is handled by removing the pin in settings, away from the moment of connecting. The data plane serves the renderer page from the same loopback listener as the socket, which makes Origin predictable -- always http://127.0.0.1:{port} -- where a WebView virtual-host mapping would give a different origin per backend and nothing to validate. The token is substituted at serve time, so it never touches disk and never appears in a URL. Being clear about what that buys: not protection from a process running as this user, which can read our memory anyway, but from a page in the user's browser attempting WebSocket connections to loopback ports, which is a real and routine thing. Two bugs the tests caught. The accept loop handled connections serially, so an upgraded WebSocket parked it inside the receive loop and every later request went unanswered -- the page's own script among them. The suite hung rather than failed, which is how I found it. And SHA-1 is unavoidable here: RFC 6455 mandates it for Sec-WebSocket-Accept, where it authenticates nothing. Suppressed narrowly with that reasoning; the alternative, HttpListener.AcceptWebSocketAsync, throws PlatformNotSupportedException off Windows. |