Skip to content

Commit 2cc6ba0

Browse files
authored
fix(perf): binary WebSocket transport and backpressure fix (#479)
* fix(perf): binary WebSocket transport and backpressure fix (#478) Fix backpressure triggering on every SSH chunk regardless of buffer state. The previous implementation used an incorrect heuristic (eventNames().length * highWaterMark) that always exceeded the threshold. Replace with actual bufferedAmount reads from the underlying WebSocket, with high/low water mark hysteresis and drain-based resume. Emit raw Buffer from SSH2 shell stream instead of converting to UTF-8 string. Socket.IO 4.x sends Buffers as binary WebSocket frames, eliminating per-chunk toString('utf8') overhead and JSON string escaping. The exec channel continues to emit strings as it accumulates output before sending. Bump webssh2_client to 3.2.2 which handles the binary data event. Server and client must be updated together — the new server is not backwards compatible with older clients. Results: CPU on cat /dev/urandom | hexdump -c dropped from 100% to ~25%. Normal interactive usage no longer triggers spurious pause/resume. Closes #478 * refactor(test): extract shared helpers to reduce duplication Extract setupShellFlow() and createMutableBufferContext() helpers from repeated test setup blocks in stream-backpressure tests.
1 parent ccaa513 commit 2cc6ba0

7 files changed

Lines changed: 523 additions & 42 deletions

File tree

DOCS/api/WEBSOCKET-API.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,10 @@ The server uses Socket.IO for real-time communication. Connect to the WebSocket
4343

4444
4. `data`
4545
- Emitted when there's output from the SSH session.
46-
- Payload: `string` (UTF-8 encoded terminal output)
46+
- Payload: `string | Buffer`
47+
- **Shell data** is emitted as a raw `Buffer`. Socket.IO 4.x automatically sends Buffers as WebSocket binary frames, avoiding the overhead of UTF-8 string conversion and JSON serialization. The client receives this as an `ArrayBuffer`.
48+
- **Exec data** (from the `exec` channel) is emitted as a `string` (UTF-8 encoded), since exec output is already a string from SSH2 and is not a high-throughput path.
49+
- Client handling: Convert `ArrayBuffer` to `Uint8Array` and pass directly to `xterm.js` `Terminal.write()`, which natively accepts `Uint8Array`. Only decode to string when text is needed (e.g., session logging).
4750
4851
5. `ssherror`
4952
- Emitted when an SSH-related error occurs.

DOCS/architecture/CLIENT.md

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ The client and server communicate using Socket.IO events:
7575

7676
| Event | Description | Payload |
7777
|-------|-------------|---------|
78-
| `data` | Terminal output | `string` |
78+
| `data` | Terminal output | `string \| ArrayBuffer` (binary for shell, string for exec) |
7979
| `connect` | Connection established | - |
8080
| `disconnect` | Connection lost | `{ reason }` |
8181
| `error` | Error message | `{ message, type }` |
@@ -113,9 +113,14 @@ const socket = io({
113113
transports: ['websocket', 'polling']
114114
});
115115

116-
// Event handling
116+
// Event handling — shell data arrives as binary (ArrayBuffer),
117+
// exec data arrives as string. xterm.js accepts both Uint8Array and string.
117118
socket.on('data', (data) => {
118-
term.write(data);
119+
if (data instanceof ArrayBuffer) {
120+
term.write(new Uint8Array(data));
121+
} else {
122+
term.write(data);
123+
}
119124
});
120125

121126
term.onData((data) => {
@@ -245,7 +250,7 @@ term.loadAddon(searchAddon);
245250

246251
### WebSocket Optimization
247252

248-
- **Binary frames** for reduced overhead
253+
- **Binary frames** for shell data — the server emits raw `Buffer` objects for shell output, which Socket.IO sends as binary WebSocket frames. This bypasses UTF-8 string conversion and JSON serialization on the server, and the client passes the resulting `Uint8Array` directly to xterm.js without decoding. String decoding is deferred to the session logger only when logging is active.
249254
- **Compression** enabled when supported
250255
- **Reconnection logic** with exponential backoff
251256
- **Connection pooling** for multiple sessions

app/socket/adapters/service-socket-terminal.ts

Lines changed: 127 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,75 @@ interface TerminalConfig {
1515
env: Record<string, string>
1616
}
1717

18+
/** Mutable state for SSH-to-WebSocket backpressure control */
19+
interface BackpressureState {
20+
paused: boolean
21+
checkTimerId: ReturnType<typeof setTimeout> | null
22+
}
23+
24+
const LOW_WATER_MARK_DIVISOR = 4
25+
26+
/**
27+
* Safely reads bufferedAmount from the ws WebSocket via the Engine.IO
28+
* transport chain. Returns null when unavailable (polling transport,
29+
* access failure, or during transport upgrade).
30+
*/
31+
export function getWebSocketBufferedBytes(
32+
socket: AdapterContext['socket']
33+
): number | null {
34+
try {
35+
// Access Engine.IO internals via unknown to avoid coupling to private types
36+
// while remaining defensive at runtime
37+
const conn: unknown = socket.conn
38+
if (typeof conn !== 'object' || conn === null) {
39+
return null
40+
}
41+
const transport: unknown = (conn as Record<string, unknown>)['transport']
42+
if (typeof transport !== 'object' || transport === null) {
43+
return null
44+
}
45+
const transportRecord = transport as Record<string, unknown>
46+
if (transportRecord['name'] !== 'websocket') {
47+
return null
48+
}
49+
const wsSocket: unknown = transportRecord['socket']
50+
if (typeof wsSocket !== 'object' || wsSocket === null) {
51+
return null
52+
}
53+
const amount: unknown = (wsSocket as Record<string, unknown>)['bufferedAmount']
54+
if (typeof amount !== 'number') {
55+
return null
56+
}
57+
return amount
58+
} catch {
59+
return null
60+
}
61+
}
62+
63+
/**
64+
* Pure decision function for backpressure control.
65+
* Returns 'pause' when buffer exceeds high water mark,
66+
* 'resume' when buffer drops below low water mark (HWM / 4),
67+
* or 'none' when no action is needed.
68+
*/
69+
export function computeBackpressureAction(
70+
bufferedBytes: number | null,
71+
highWaterMark: number,
72+
currentlyPaused: boolean
73+
): 'pause' | 'resume' | 'none' {
74+
if (bufferedBytes === null) {
75+
return 'none'
76+
}
77+
const lowWaterMark = Math.floor(highWaterMark / LOW_WATER_MARK_DIVISOR)
78+
if (!currentlyPaused && bufferedBytes >= highWaterMark) {
79+
return 'pause'
80+
}
81+
if (currentlyPaused && bufferedBytes < lowWaterMark) {
82+
return 'resume'
83+
}
84+
return 'none'
85+
}
86+
1887
export class ServiceSocketTerminal {
1988
constructor(private readonly context: AdapterContext) {}
2089

@@ -180,12 +249,61 @@ export class ServiceSocketTerminal {
180249

181250
// Get rate limit configuration (0 = unlimited)
182251
const rateLimitBytesPerSec = this.context.config.ssh.outputRateLimitBytesPerSec ?? 0
183-
const highWaterMark = this.context.config.ssh.socketHighWaterMark ?? 16384
184252

185253
let rateLimitState:
186254
| { bytesInWindow: number; windowStart: number; paused: boolean }
187255
| null = rateLimitBytesPerSec > 0 ? { bytesInWindow: 0, windowStart: Date.now(), paused: false } : null
188256

257+
// Backpressure state for WebSocket outbound buffer
258+
const highWaterMark = this.context.config.ssh.socketHighWaterMark ?? 16384
259+
const backpressure: BackpressureState = { paused: false, checkTimerId: null }
260+
261+
const clearResumeSchedule = (): void => {
262+
if (backpressure.checkTimerId !== null) {
263+
clearTimeout(backpressure.checkTimerId)
264+
backpressure.checkTimerId = null
265+
}
266+
this.context.socket.conn.removeListener('drain', onDrainCheck)
267+
}
268+
269+
const onDrainCheck = (): void => {
270+
checkResume()
271+
}
272+
273+
const checkResume = (): void => {
274+
const buffered = getWebSocketBufferedBytes(this.context.socket)
275+
const action = computeBackpressureAction(buffered, highWaterMark, backpressure.paused)
276+
if (action === 'resume') {
277+
backpressure.paused = false
278+
clearResumeSchedule()
279+
stream.resume()
280+
this.context.debug('Backpressure relieved, resuming SSH stream (buffered=%d)', buffered)
281+
} else if (backpressure.paused) {
282+
// Still above low water mark — re-register for next drain + safety timer
283+
scheduleResumeCheck()
284+
}
285+
}
286+
287+
const scheduleResumeCheck = (): void => {
288+
clearResumeSchedule()
289+
this.context.socket.conn.once('drain', onDrainCheck)
290+
backpressure.checkTimerId = setTimeout(() => {
291+
backpressure.checkTimerId = null
292+
checkResume()
293+
}, 50)
294+
}
295+
296+
const checkBackpressure = (): void => {
297+
const buffered = getWebSocketBufferedBytes(this.context.socket)
298+
const action = computeBackpressureAction(buffered, highWaterMark, backpressure.paused)
299+
if (action === 'pause') {
300+
backpressure.paused = true
301+
stream.pause()
302+
this.context.debug('Backpressure detected, pausing SSH stream (buffered=%d)', buffered)
303+
scheduleResumeCheck()
304+
}
305+
}
306+
189307
stream.on('data', (chunk: Buffer) => {
190308
const chunkSize = chunk.length
191309

@@ -215,7 +333,9 @@ export class ServiceSocketTerminal {
215333
rateLimitState.bytesInWindow = 0
216334
rateLimitState.windowStart = Date.now()
217335
rateLimitState.paused = false
218-
stream.resume()
336+
if (!backpressure.paused) {
337+
stream.resume()
338+
}
219339
this.context.debug('Rate limit window reset, resuming SSH stream')
220340
}
221341
}, delayMs)
@@ -226,23 +346,16 @@ export class ServiceSocketTerminal {
226346
rateLimitState.bytesInWindow += chunkSize
227347
}
228348

229-
// Check Socket.IO buffer backpressure
230-
const bufferSize = this.context.socket.eventNames().length * highWaterMark
231-
if (bufferSize > highWaterMark * 2) {
232-
stream.pause()
233-
this.context.debug('Socket.IO buffer high, pausing SSH stream')
349+
this.context.socket.emit(SOCKET_EVENTS.SSH_DATA, chunk)
234350

235-
// Resume when socket drains (use setImmediate to check on next tick)
236-
setImmediate(() => {
237-
stream.resume()
238-
this.context.debug('Socket.IO buffer drained, resuming SSH stream')
239-
})
351+
// Check backpressure after emit (standard Node.js streams pattern)
352+
if (!backpressure.paused) {
353+
checkBackpressure()
240354
}
241-
242-
this.context.socket.emit(SOCKET_EVENTS.SSH_DATA, chunk.toString('utf8'))
243355
})
244356

245357
stream.on('close', () => {
358+
clearResumeSchedule()
246359
rateLimitState = null
247360
this.context.socket.emit(SOCKET_EVENTS.SSH_ERROR, VALIDATION_MESSAGES.CONNECTION_CLOSED)
248361
this.context.socket.disconnect()

app/types/contracts/v1/socket.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ export type AuthenticationEvent =
238238
// Server → Client events
239239
export interface ServerToClientEvents {
240240
// Stream data
241-
data: (chunk: string) => void
241+
data: (chunk: string | Buffer) => void
242242
// Auth flow
243243
authentication: (payload: AuthenticationEvent) => void
244244
authFailure: (payload: { error: string; method: string }) => void

package-lock.json

Lines changed: 9 additions & 19 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@
4747
"socket.io": "^4.8.1",
4848
"ssh2": "1.17",
4949
"validator": "^13.15.23",
50-
"webssh2_client": "^3.2.0",
50+
"webssh2_client": "^3.2.2",
5151
"zod": "^4.1.12"
5252
},
5353
"scripts": {

0 commit comments

Comments
 (0)