Improve port-forwarding resilience under bursty multi-port load

[DavidObando]

Context

Follow-up to #138, which fixed unhandled 'error' events on duplex streams inside StreamForwarder that were crashing Node host processes (extension host using the Codespaces extension, Node apps using TunnelRelayTunnelClient, etc.) under bursty multi-port forwarding.

While reviewing #138 we found several related issues that PR did not cover. This PR addresses the ones that are small, tightly related, and don't change public API or wire-protocol behavior. Two larger follow-ups (TCP socket options on forwarded sockets, and SshStream flow-control improvements) are deferred to their own discussions.

Changes

1. Bug: RemotePortForwarder.forwardChannel constructed a StreamForwarder even after the local TCP connect failed

The previous code set request.failureReason = connectFailed but then fell through and built a forwarder around a destroyed socket and a still-open SSH channel. Pre-#138 this was a likely crash trigger; post-#138 it created a self-disposing zombie. Now we return early and destroy() the forwardedStream so the underlying SSH channel is released.

2. Memory leak: pfs.streamForwarders was append-only

PortForwardingService.streamForwarders was an array that everyone pushed to but no one removed from. Disposed forwarders were retained for the lifetime of the session, holding references to a Socket and an SshStream. Under the bursty connect/disconnect workload that motivated #138, this is a real leak.

Now it's a Set<StreamForwarder> and StreamForwarder accepts an optional onDisposed callback that the PFS uses to splice itself out.

3. Pre-construction unhandled 'error' window on LocalPortForwarder.acceptConnection

Between accepting a TCP connection and constructing the StreamForwarder (which now installs the listeners from #138), the code awaits pfs.openChannel(...) and pfs.forwardedPortConnecting(...). A peer reset during that await window emits 'error' on a listener-less socket — same crash class as #138, just earlier in the lifecycle. Fixed by attaching a temporary 'error' listener on accept and removing/transferring it when the forwarder takes over.

Also: when the connecting event handler rejects the connection (returns falsy), the previous code returned without destroying the socket — leaked file descriptor. Now destroyed.

4. Pre-construction unhandled 'error' window on RemotePortForwarder.forwardChannel

Symmetric fix on the SSH-stream side: attach a temporary listener on the SshStream while we await forwardedPortConnecting and the local net.createConnection. A remote channel reset in that window would otherwise emit an unhandled 'error' from the SshStream.

5. SshStream.destroy swallowed channel.close() errors

Was void this.channel.close().catch(); — silently dropped any rejection. Now traces them at Warning so the new error path doesn't lose diagnostic information.

Deferred

• TCP socket options on forwarded sockets (the literal // TODO: Set socket options? in both forwarders) — setNoDelay/setKeepAlive are clearly desirable for bursty workloads, but choosing keepalive intervals, deciding on configurability vs. hardcoded defaults, and checking C# parity needs its own discussion. Detailed notes have been written up separately.
• SshStream flow-control improvements — the existing self-aware comment in sshStream.ts about choppy SSH window adjustment is real, but addressing it requires either decoupling window adjust from push() backpressure (lower-risk patch) or replacing Duplex with a custom pull-based implementation (ideal but invasive). Detailed notes have been written up separately.
• Migrating to stream.pipeline() in StreamForwarder — would simplify the manual error/teardown wiring, but changes teardown semantics (pipeline destroys both streams on error) and warrants its own PR with focused testing.

Behavior

Happy path is unchanged. On the error path: forwarders self-dispose, are removed from the PFS collection, and warning traces include the side (local / remote) and reason. No public API changes, no wire-protocol changes.

Impact on Codespaces clients

• Browser: leak fix and SshStream.destroy tracing apply; the new error windows mostly close pre-existing latent races. No net.Socket paths run.
• VS Code Codespaces extension (Node extension host): items 1, 2, 3, 4 directly address remaining unhandled-'error'/leak scenarios that Fix StreamForwarder crashing host process on unhandled socket errors #138 didn't cover. Combined with Fix StreamForwarder crashing host process on unhandled socket errors #138, the extension host should now tolerate the full bursty multi-port-forward + reconnect workload without process termination or unbounded memory growth in the PFS.

Verification

• npm run compile ✅
• npm run eslint ✅
• npm run test-ts ✅

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Improve resilience and lifecycle management of SSH TCP port-forwarding under bursty connect/disconnect and multi-port load by closing pre-construction unhandled 'error' windows, preventing leaked forwarders, and improving diagnostics.

Changes:

• Add pre-forwarding temporary 'error' handlers for accepted sockets and SSH streams; ensure resources are destroyed on rejection/failure paths.
• Replace append-only forwarder tracking with Set<StreamForwarder> and add an onDisposed callback to support removal on disposal.
• Add unit tests for StreamForwarder data forwarding and error/disposal behavior; add warning trace on SshStream.destroy() close failures.

Show a summary per file

File	Description
test/ts/ssh-test/streamForwarderTests.ts	Adds StreamForwarder unit tests covering data piping, error-triggered disposal, and disposal callback behavior.
src/ts/ssh/sshStream.ts	Traces channel.close() failures during destroy() instead of silently swallowing them.
src/ts/ssh-tcp/services/streamForwarder.ts	Adds onDisposed callback + isDisposed and ensures callback errors are handled during disposal.
src/ts/ssh-tcp/services/remotePortForwarder.ts	Adds temporary SSH stream error handler during setup, fixes failure-path teardown, and integrates forwarder removal via callback + Set.
src/ts/ssh-tcp/services/portForwardingService.ts	Changes streamForwarders to a Set and adds a removal callback; clears set on dispose.
src/ts/ssh-tcp/services/localPortForwarder.ts	Adds temporary socket error handler during setup, destroys socket on rejection, and integrates forwarder removal via callback + Set.
src/ts/ssh-tcp/index.ts	Exports StreamForwarder from the package entrypoint (public surface change).

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 7/7 changed files
• Comments generated: 4