From fb04edc4974dcda657ce6279f0be25a9cc2bf7e6 Mon Sep 17 00:00:00 2001 From: ZacharyZcR Date: Wed, 14 Oct 2026 08:12:16 +0800 Subject: [PATCH] fix: keep concurrent ssh pool connects from overwriting each other (#1619) --- src/backend/hosts/ssh-connection-pool.ts | 12 +++-- .../tests/hosts/ssh-connection-pool.test.ts | 46 +++++++++++++++++++ 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/src/backend/hosts/ssh-connection-pool.ts b/src/backend/hosts/ssh-connection-pool.ts index d6816b0a9..8527d12ea 100644 --- a/src/backend/hosts/ssh-connection-pool.ts +++ b/src/backend/hosts/ssh-connection-pool.ts @@ -86,7 +86,6 @@ export class SSHConnectionPool { private async createPooledClient( key: string, factory: () => Promise, - existing: PooledConnection[], ): Promise { const generation = this.generations.get(key) || 0; this.pendingConnections.set( @@ -109,8 +108,11 @@ export class SSHConnectionPool { inUse: true, hostKey: key, }; - existing.push(pooled); - this.connections.set(key, existing); + // Re-read after the await: concurrent connects and removals may have + // replaced this key's array in the meantime. + const connections = this.connections.get(key) ?? []; + connections.push(pooled); + this.connections.set(key, connections); client.on("end", () => { this.removeConnection(key, client); @@ -187,7 +189,7 @@ export class SSHConnectionPool { if (queue.length === 0) this.waiters.delete(key); else this.waiters.set(key, queue); clearTimeout(waiter.timer); - this.createPooledClient(key, waiter.factory, connections) + this.createPooledClient(key, waiter.factory) .then(waiter.resolve) .catch(waiter.reject); return; @@ -225,7 +227,7 @@ export class SSHConnectionPool { } if (this.hasCapacity(key, connections)) { - return this.createPooledClient(key, factory, connections); + return this.createPooledClient(key, factory); } return this.enqueueWaiter(key, factory); diff --git a/src/backend/tests/hosts/ssh-connection-pool.test.ts b/src/backend/tests/hosts/ssh-connection-pool.test.ts index ba3f4700b..5c9c9f322 100644 --- a/src/backend/tests/hosts/ssh-connection-pool.test.ts +++ b/src/backend/tests/hosts/ssh-connection-pool.test.ts @@ -42,6 +42,52 @@ describe("SSHConnectionPool", () => { pool.destroy(); }); + it("tracks every client from concurrent connects to a new host", async () => { + const pool = new SSHConnectionPool(); + const clients = Array.from({ length: 3 }, deferredClient); + let factoryCalls = 0; + const factory = vi.fn(() => clients[factoryCalls++].promise); + + const requests = Array.from({ length: 3 }, () => + pool.getConnection("new-host", factory), + ); + for (const pending of clients) pending.resolve(pending.client); + const connected = await Promise.all(requests); + + for (const client of connected) pool.releaseConnection("new-host", client); + const reused = await Promise.all( + Array.from({ length: 3 }, () => pool.getConnection("new-host", factory)), + ); + expect(new Set(reused)).toEqual(new Set(connected)); + expect(factory).toHaveBeenCalledTimes(3); + + pool.clearKeyConnections("new-host"); + for (const pending of clients) { + expect(pending.client.end).toHaveBeenCalledOnce(); + } + pool.destroy(); + }); + + it("does not resurrect a connection removed while another connects", async () => { + const pool = new SSHConnectionPool(); + const first = deferredClient(); + const second = deferredClient(); + first.resolve(first.client); + await pool.getConnection("host", () => first.promise); + + const request = pool.getConnection("host", () => second.promise); + first.client.emit("close"); + second.resolve(second.client); + await expect(request).resolves.toBe(second.client); + + pool.releaseConnection("host", first.client); + pool.releaseConnection("host", second.client); + await expect( + pool.getConnection("host", () => deferredClient().promise), + ).resolves.toBe(second.client); + pool.destroy(); + }); + it("discards a pending connection when its host pool is cleared", async () => { const pool = new SSHConnectionPool(); const pending = deferredClient();