ssh: disable Nagle's algorithm on accepted connections (#1531)

On Node, a server socket keeps Nagle's algorithm enabled. A frame
written as more than one small packet then waits for the client's
delayed ACK. On Linux that adds about 40 ms to each reply. Bun's sockets
are not affected.

Call setNoDelay(true) on each accepted ssh2 connection, after the close
and error handlers are registered, so a failure is reported through the
normal cleanup path.

Loopback ssh2 probe, two small writes per reply, client with TCP_NODELAY
(as interactive OpenSSH sets it), median of 60 replies:

  node 26.4  off 41.011 ms  on 0.107 ms
  node 26.7  off 41.003 ms  on 0.106 ms
  bun 1.4    off  0.048 ms  on 0.045 ms
This commit is contained in:
Simon Klee authored and GitHub committed 2026-09-25 09:39:00 +02:00
1 parent 4b1474e009
commit 28b27a587e
3 files changed
+88

No files matched your search

+3
View File
@@ -145,6 +145,9 @@ export function createConnectionHandler(dependencies: ConnectionDependencies): {
clients.delete(client)
})
client.on("error", (err: Error) => safe.report(err))
// Nagle would hold a frame's later packets until the client's delayed ACK. Set it
// after the close/error handlers so a failure takes the normal cleanup path.
void safe(() => client.setNoDelay(true))
}
const closeAll = async () => {
+5
View File
@@ -8,4 +8,9 @@ declare module "ssh2" {
/** The client's requested `TERM`. Sent at runtime; omitted by @types/ssh2. */
term?: string
}
interface Connection {
/** Calls `setNoDelay()` on the underlying socket. Documented by ssh2; omitted by @types/ssh2. */
setNoDelay(noDelay?: boolean): this
}
}
@@ -5,12 +5,87 @@ import { createConnectionHandler } from "../../connection.js"
import { createSafeInvoke } from "../../safe.js"
import { deferred } from "../support.js"
test("each accepted connection disables Nagle's algorithm", async () => {
const errors: unknown[] = []
const calls: unknown[] = []
const client = Object.assign(new EventEmitter(), {
setNoDelay(enabled: boolean) {
calls.push(enabled)
},
end() {},
_sock: { destroy() {} },
}) as unknown as Connection
const handler = createConnectionHandler({
authenticator: {
advertisedMethods: () => ["none"],
authenticate: async () => ({ type: "reject", methods: ["none"] }),
handle: async () => ({ type: "reject", methods: ["none"] }),
},
middlewares: [],
handler: () => {},
safe: createSafeInvoke((error) => errors.push(error)),
idleTimeoutMs: undefined,
maxTimeoutMs: undefined,
sessionLimits: { perConnection: 1, global: 1 },
})
try {
handler.onConnection(client, { ip: "127.0.0.1", port: 1234 } as ClientInfo)
expect(calls).toEqual([true])
expect(errors).toEqual([])
} finally {
await handler.closeAll()
}
})
test("a no-delay transport failure preserves connection cleanup", async () => {
const failure = new Error("custom transport failure")
const errors: unknown[] = []
let endCalls = 0
let destroyCalls = 0
const client = Object.assign(new EventEmitter(), {
setNoDelay(this: EventEmitter) {
this.emit("close")
throw failure
},
end() {
endCalls++
},
_sock: {
destroy() {
destroyCalls++
},
},
}) as unknown as Connection
const handler = createConnectionHandler({
authenticator: {
advertisedMethods: () => ["none"],
authenticate: async () => ({ type: "reject", methods: ["none"] }),
handle: async () => ({ type: "reject", methods: ["none"] }),
},
middlewares: [],
handler: () => {},
safe: createSafeInvoke((error) => errors.push(error)),
idleTimeoutMs: undefined,
maxTimeoutMs: undefined,
sessionLimits: { perConnection: 1, global: 1 },
})
try {
expect(() => handler.onConnection(client, { ip: "127.0.0.1", port: 1234 } as ClientInfo)).not.toThrow()
expect(errors).toEqual([failure])
} finally {
await handler.closeAll()
}
expect(endCalls).toBe(0)
expect(destroyCalls).toBe(0)
})
test("an authentication decision is ignored after the connection closes", async () => {
const started = deferred<void>()
const decision = deferred<void>()
let accepts = 0
let rejects = 0
const client = Object.assign(new EventEmitter(), {
setNoDelay() {},
end() {},
_sock: { destroy() {} },
}) as unknown as Connection
@@ -64,6 +139,7 @@ test("closeAll waits for a logically closed bridge to finish draining", async ()
const sshSession = new EventEmitter()
let clientEndCalls = 0
const client = Object.assign(new EventEmitter(), {
setNoDelay() {},
end() {
clientEndCalls++
},
@@ -120,6 +196,7 @@ test("closeAll force-closes a client that never drains", async () => {
const sshSession = new EventEmitter()
let socketDestroyCalls = 0
const client = Object.assign(new EventEmitter(), {
setNoDelay() {},
end() {},
_sock: {
destroy() {
@@ -169,6 +246,7 @@ test("closeAll rejects shells requested after shutdown begins", async () => {
close() {},
})
const client = Object.assign(new EventEmitter(), {
setNoDelay() {},
end() {},
_sock: { destroy() {} },
}) as unknown as Connection
@@ -227,6 +305,7 @@ test("closeAll rejects shells requested after shutdown begins", async () => {
test("a bridge setup failure releases reserved capacity", () => {
const errors: unknown[] = []
const client = Object.assign(new EventEmitter(), {
setNoDelay() {},
end() {},
_sock: { destroy() {} },
}) as unknown as Connection
@@ -285,6 +364,7 @@ test("per-connection and global limits reject before accepting a shell", async (
const connect = () => {
const client = Object.assign(new EventEmitter(), {
setNoDelay() {},
end() {},
_sock: { destroy() {} },
}) as unknown as Connection