Skip to content

Commit 2beb4f6

Browse files
committed
fix(transport): fix InMemoryTransport close() deadlock
- Make close() idempotent with _isClosed flag - Prevent infinite recursion by clearing _otherTransport before calling peer.close() - Ensure both transports' onclose callbacks fire - Add tests for idempotency, concurrent closes, and post-close behavior Fixes deadlock where closing one transport would not properly close its peer, causing promises to hang indefinitely.
1 parent 0551cc5 commit 2beb4f6

File tree

2 files changed

+69
-4
lines changed

2 files changed

+69
-4
lines changed

src/inMemory.test.ts

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,4 +118,59 @@ describe("InMemoryTransport", () => {
118118
await serverTransport.start();
119119
expect(receivedMessage).toEqual(message);
120120
});
121+
122+
test("should handle double close idempotently", async () => {
123+
let clientCloseCount = 0;
124+
let serverCloseCount = 0;
125+
126+
clientTransport.onclose = () => {
127+
clientCloseCount++;
128+
};
129+
130+
serverTransport.onclose = () => {
131+
serverCloseCount++;
132+
};
133+
134+
await clientTransport.close();
135+
await clientTransport.close(); // Second close should be idempotent
136+
137+
expect(clientCloseCount).toBe(1);
138+
expect(serverCloseCount).toBe(1);
139+
});
140+
141+
test("should handle concurrent close from both sides", async () => {
142+
let clientCloseCount = 0;
143+
let serverCloseCount = 0;
144+
145+
clientTransport.onclose = () => {
146+
clientCloseCount++;
147+
};
148+
149+
serverTransport.onclose = () => {
150+
serverCloseCount++;
151+
};
152+
153+
// Close both sides concurrently
154+
await Promise.all([
155+
clientTransport.close(),
156+
serverTransport.close()
157+
]);
158+
159+
expect(clientCloseCount).toBe(1);
160+
expect(serverCloseCount).toBe(1);
161+
});
162+
163+
test("should reject send after close from either side", async () => {
164+
await serverTransport.close();
165+
166+
// Both sides should reject sends
167+
await expect(
168+
clientTransport.send({ jsonrpc: "2.0", method: "test", id: 1 })
169+
).rejects.toThrow("Not connected");
170+
171+
await expect(
172+
serverTransport.send({ jsonrpc: "2.0", method: "test", id: 2 })
173+
).rejects.toThrow("Not connected");
174+
});
175+
121176
});

src/inMemory.ts

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ interface QueuedMessage {
1313
export class InMemoryTransport implements Transport {
1414
private _otherTransport?: InMemoryTransport;
1515
private _messageQueue: QueuedMessage[] = [];
16+
private _isClosed = false;
17+
private _closePromise?: Promise<void>;
1618

1719
onclose?: () => void;
1820
onerror?: (error: Error) => void;
@@ -39,10 +41,18 @@ export class InMemoryTransport implements Transport {
3941
}
4042

4143
async close(): Promise<void> {
42-
const other = this._otherTransport;
43-
this._otherTransport = undefined;
44-
await other?.close();
45-
this.onclose?.();
44+
if (this._isClosed) return this._closePromise ?? Promise.resolve();
45+
46+
this._isClosed = true;
47+
this._closePromise = (async () => {
48+
const peer = this._otherTransport;
49+
this._otherTransport = undefined; // Prevent infinite recursion
50+
51+
this.onclose?.();
52+
await peer?.close();
53+
})();
54+
55+
return this._closePromise;
4656
}
4757

4858
/**

0 commit comments

Comments
 (0)