Skip to content

Commit 85106aa

Browse files
bajrangCoderbajrangCoder
authored
fix(lsp): preserve websocket clients across tab switches (#2764)
* fix(lsp): preserve websocket clients across tab switches Keep externally managed WebSocket runtimes alive when editor views detach, retain eager cleanup for owned runtimes * fix(lsp): dispose idle clients after grace period
1 parent de25d05 commit 85106aa

3 files changed

Lines changed: 330 additions & 2 deletions

File tree

‎src/cm/lsp/clientManager.ts‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ import type {
5353
} from "./types";
5454
import AcodeWorkspace from "./workspace";
5555

56+
export const DEFAULT_CLIENT_IDLE_GRACE_PERIOD_MS = 15_000;
57+
5658
export const lspCompletionEnabled = Facet.define<boolean, boolean>({
5759
// File-level marker used by the autocomplete override path. If any attached
5860
// server exposes completion, keep the shared LSP completion source available.
@@ -1136,12 +1138,20 @@ export class LspClientManager {
11361138
const uriAliases = new Map<string, string>();
11371139
const effectiveRoot = normalizedRootUri ?? originalRootUri ?? null;
11381140
let disposed = false;
1141+
let idleTimer: ReturnType<typeof setTimeout> | null = null;
1142+
1143+
const cancelIdleTimer = (): void => {
1144+
if (idleTimer === null) return;
1145+
clearTimeout(idleTimer);
1146+
idleTimer = null;
1147+
};
11391148

11401149
const attach = (
11411150
uri: string,
11421151
view: EditorView,
11431152
aliases: string[] = [],
11441153
): void => {
1154+
cancelIdleTimer();
11451155
const existing = fileRefs.get(uri) ?? new Set();
11461156
existing.add(view);
11471157
fileRefs.set(uri, existing);
@@ -1165,6 +1175,7 @@ export class LspClientManager {
11651175
const dispose = async (): Promise<void> => {
11661176
if (disposed) return;
11671177
disposed = true;
1178+
cancelIdleTimer();
11681179
disposePullDiagnostics(client);
11691180
this.#clients.delete(key);
11701181
for (const views of fileRefs.values()) {
@@ -1206,14 +1217,24 @@ export class LspClientManager {
12061217
}
12071218
}
12081219

1209-
if (!fileRefs.size) {
1220+
if (fileRefs.size || idleTimer !== null) return;
1221+
1222+
const configuredGracePeriod = this.options.clientIdleGracePeriodMs;
1223+
const gracePeriod =
1224+
typeof configuredGracePeriod === "number" &&
1225+
Number.isFinite(configuredGracePeriod)
1226+
? Math.max(0, configuredGracePeriod)
1227+
: DEFAULT_CLIENT_IDLE_GRACE_PERIOD_MS;
1228+
idleTimer = setTimeout(() => {
1229+
idleTimer = null;
1230+
if (disposed || fileRefs.size) return;
12101231
this.options.onClientIdle?.({
12111232
server,
12121233
client,
12131234
rootUri: effectiveRoot,
12141235
dispose,
12151236
});
1216-
}
1237+
}, gracePeriod);
12171238
};
12181239

12191240
return {

‎src/cm/lsp/types.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -411,6 +411,8 @@ export interface ClientManagerOptions {
411411
displayFile?: (uri: string) => Promise<EditorView | null>;
412412
openFile?: (uri: string) => Promise<EditorView | null>;
413413
resolveLanguageId?: (uri: string) => string | null;
414+
/** Delay before an unreferenced client is reported as idle. */
415+
clientIdleGracePeriodMs?: number;
414416
onClientIdle?: (info: ClientIdleInfo) => void;
415417
allowNonTerminalWorkspace?: boolean;
416418
}

‎tests/unit/lspExternalWebSocketLifecycle.test.js‎

Lines changed: 305 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,305 @@
1+
// @vitest-environment happy-dom
2+
3+
import {StateEffect} from "@codemirror/state";
4+
import {EditorView} from "@codemirror/view";
5+
import {afterEach, beforeEach, describe, expect, it, vi} from "vitest";
6+
7+
const registry = vi.hoisted(() => ({servers: []}));
8+
9+
// Keep the lifecycle test independent of app UI modules that use JSX in .js
10+
// files, which Vitest's native import analysis does not transform.
11+
vi.mock("cm/lsp/serverRegistry", () => ({
12+
default: {
13+
getServersForLanguage: (languageId) =>
14+
registry.servers.filter(
15+
(server) => server.enabled !== false && server.languages.includes(languageId),
16+
),
17+
},
18+
}));
19+
20+
vi.mock("components/lspStatusBar", () => ({
21+
default: {
22+
show: vi.fn(),
23+
update: vi.fn(),
24+
hideById: vi.fn(),
25+
},
26+
}));
27+
28+
vi.mock("components/settingsPage", () => ({default: vi.fn()}));
29+
vi.mock("components/checkbox", () => ({
30+
default: vi.fn(),
31+
updateSwitchHandle: vi.fn(),
32+
}));
33+
34+
vi.mock("lib/notificationManager", () => ({
35+
default: {add: vi.fn()},
36+
}));
37+
38+
vi.mock("lib/settings", () => ({
39+
default: {value: {lsp: {}}},
40+
}));
41+
42+
vi.mock("cm/lsp/diagnostics", () => ({
43+
clearDiagnosticsEffect: StateEffect.define(),
44+
disposePullDiagnostics: vi.fn(),
45+
lspDiagnosticsAutoSyncExtension: () => [],
46+
}));
47+
48+
vi.mock("cm/lsp/documentColors", () => ({
49+
documentColorsExtension: () => [],
50+
}));
51+
52+
vi.mock("cm/lsp/formattingSupport", () => ({
53+
supportsBuiltinFormatting: () => false,
54+
}));
55+
56+
vi.mock("cm/lsp/inlayHints", () => ({
57+
inlayHintsExtension: () => [],
58+
}));
59+
60+
vi.mock("cm/lsp/logs", () => ({addLspLog: vi.fn()}));
61+
62+
vi.mock("cm/lsp/tooltipExtensions", () => ({
63+
hoverTooltips: () => [],
64+
resolveLspHoverHighlightLanguage: vi.fn(),
65+
signatureHelp: () => [],
66+
}));
67+
68+
import {
69+
DEFAULT_CLIENT_IDLE_GRACE_PERIOD_MS,
70+
LspClientManager,
71+
} from "cm/lsp/clientManager";
72+
import {
73+
registerRuntimeProvider,
74+
unregisterRuntimeProvider,
75+
} from "cm/lsp/runtimeProviders";
76+
import externalWebSocketRuntimeProvider from "cm/lsp/runtimes/externalWebSocket";
77+
78+
const SERVER_ID = "external-websocket-lifecycle-test";
79+
const LANGUAGE_ID = "external-websocket-lifecycle-test";
80+
const TRANSPORT_RUNTIME_ID = "transport-lifecycle-test";
81+
82+
class TestWebSocket {
83+
static CONNECTING = 0;
84+
static OPEN = 1;
85+
static CLOSING = 2;
86+
static CLOSED = 3;
87+
static instances = [];
88+
89+
readyState = TestWebSocket.CONNECTING;
90+
onopen = null;
91+
onmessage = null;
92+
onerror = null;
93+
onclose = null;
94+
sent = [];
95+
closeCalls = 0;
96+
97+
constructor(url) {
98+
this.url = url;
99+
TestWebSocket.instances.push(this);
100+
queueMicrotask(() => {
101+
this.readyState = TestWebSocket.OPEN;
102+
this.onopen?.({type: "open"});
103+
});
104+
}
105+
106+
send(data) {
107+
if (this.readyState !== TestWebSocket.OPEN) {
108+
throw new Error("socket is not open");
109+
}
110+
this.sent.push(data);
111+
const message = JSON.parse(data);
112+
if (message.method !== "initialize") return;
113+
queueMicrotask(() => {
114+
this.onmessage?.({
115+
data: JSON.stringify({
116+
jsonrpc: "2.0",
117+
id: message.id,
118+
result: {capabilities: {}},
119+
}),
120+
});
121+
});
122+
}
123+
124+
close(code = 1000) {
125+
this.closeCalls++;
126+
this.readyState = TestWebSocket.CLOSED;
127+
this.onclose?.({code, wasClean: code === 1000});
128+
}
129+
}
130+
131+
class TestTransport {
132+
handler = null;
133+
disposeCalls = 0;
134+
135+
send(data) {
136+
const message = JSON.parse(data);
137+
if (message.method !== "initialize") return;
138+
queueMicrotask(() => {
139+
this.handler?.(
140+
JSON.stringify({
141+
jsonrpc: "2.0",
142+
id: message.id,
143+
result: {capabilities: {}},
144+
}),
145+
);
146+
});
147+
}
148+
149+
subscribe(handler) {
150+
this.handler = handler;
151+
}
152+
153+
unsubscribe(handler) {
154+
if (this.handler === handler) this.handler = null;
155+
}
156+
157+
dispose() {
158+
this.disposeCalls++;
159+
}
160+
}
161+
162+
let originalWebSocket;
163+
let manager;
164+
let view;
165+
let testTransport;
166+
167+
beforeEach(() => {
168+
originalWebSocket = globalThis.WebSocket;
169+
globalThis.WebSocket = TestWebSocket;
170+
TestWebSocket.instances = [];
171+
registerRuntimeProvider(externalWebSocketRuntimeProvider, {replace: true});
172+
registry.servers = [
173+
{
174+
id: SERVER_ID,
175+
label: SERVER_ID,
176+
enabled: true,
177+
priority: 0,
178+
languages: [LANGUAGE_ID],
179+
transport: {
180+
kind: "websocket",
181+
url: "ws://localhost:3030",
182+
},
183+
},
184+
];
185+
view = new EditorView({doc: "fn main() {}", parent: document.body});
186+
});
187+
188+
afterEach(async () => {
189+
await manager?.dispose();
190+
view?.destroy();
191+
registry.servers = [];
192+
unregisterRuntimeProvider(TRANSPORT_RUNTIME_ID);
193+
globalThis.WebSocket = originalWebSocket;
194+
document.body.replaceChildren();
195+
vi.useRealTimers();
196+
vi.restoreAllMocks();
197+
});
198+
199+
describe("LSP client idle lifecycle", () => {
200+
it("reuses an external WebSocket client when a file attaches during the grace period", async () => {
201+
const onClientIdle = vi.fn(({dispose}) => void dispose());
202+
manager = new LspClientManager({onClientIdle});
203+
const rootUri = "file:///workspace";
204+
205+
await manager.getExtensionsForFile({
206+
uri: `${rootUri}/first.rs`,
207+
rootUri,
208+
languageId: LANGUAGE_ID,
209+
view,
210+
});
211+
vi.useFakeTimers();
212+
manager.detach(`${rootUri}/first.rs`, view);
213+
await vi.advanceTimersByTimeAsync(DEFAULT_CLIENT_IDLE_GRACE_PERIOD_MS - 1);
214+
215+
await manager.getExtensionsForFile({
216+
uri: `${rootUri}/second.rs`,
217+
rootUri,
218+
languageId: LANGUAGE_ID,
219+
view,
220+
});
221+
await vi.advanceTimersByTimeAsync(DEFAULT_CLIENT_IDLE_GRACE_PERIOD_MS + 1);
222+
223+
expect(onClientIdle).not.toHaveBeenCalled();
224+
expect(manager.getActiveClients()).toHaveLength(1);
225+
expect(TestWebSocket.instances).toHaveLength(1);
226+
expect(TestWebSocket.instances[0].closeCalls).toBe(0);
227+
});
228+
229+
it("disposes an unused external WebSocket client after the grace period", async () => {
230+
const onClientIdle = vi.fn(({dispose}) => void dispose());
231+
manager = new LspClientManager({onClientIdle});
232+
const rootUri = "file:///workspace-one";
233+
const uri = `${rootUri}/only.rs`;
234+
235+
await manager.getExtensionsForFile({
236+
uri,
237+
rootUri,
238+
languageId: LANGUAGE_ID,
239+
view,
240+
});
241+
vi.useFakeTimers();
242+
manager.detach(uri, view);
243+
await vi.advanceTimersByTimeAsync(DEFAULT_CLIENT_IDLE_GRACE_PERIOD_MS - 1);
244+
245+
expect(onClientIdle).not.toHaveBeenCalled();
246+
expect(manager.getActiveClients()).toHaveLength(1);
247+
expect(TestWebSocket.instances[0].closeCalls).toBe(0);
248+
249+
await vi.advanceTimersByTimeAsync(1);
250+
251+
expect(onClientIdle).toHaveBeenCalledOnce();
252+
expect(manager.getActiveClients()).toHaveLength(0);
253+
expect(TestWebSocket.instances[0].closeCalls).toBe(1);
254+
});
255+
256+
it("applies the same grace period to other runtimes", async () => {
257+
registerRuntimeProvider(
258+
{
259+
id: TRANSPORT_RUNTIME_ID,
260+
label: "Transport test runtime",
261+
priority: 100,
262+
canHandle: () => true,
263+
start: async () => {
264+
testTransport = new TestTransport();
265+
return {
266+
kind: "transport",
267+
providerId: TRANSPORT_RUNTIME_ID,
268+
transport: {
269+
transport: testTransport,
270+
ready: Promise.resolve(),
271+
dispose: () => testTransport.dispose(),
272+
},
273+
};
274+
},
275+
},
276+
{replace: true},
277+
);
278+
registry.servers[0].runtimes = [TRANSPORT_RUNTIME_ID];
279+
const onClientIdle = vi.fn(({dispose}) => void dispose());
280+
manager = new LspClientManager({onClientIdle});
281+
const rootUri = "file:///workspace";
282+
const uri = `${rootUri}/only.rs`;
283+
284+
await manager.getExtensionsForFile({
285+
uri,
286+
rootUri,
287+
languageId: LANGUAGE_ID,
288+
view,
289+
});
290+
vi.useFakeTimers();
291+
manager.detach(uri, view);
292+
await vi.advanceTimersByTimeAsync(DEFAULT_CLIENT_IDLE_GRACE_PERIOD_MS - 1);
293+
294+
expect(onClientIdle).not.toHaveBeenCalled();
295+
expect(manager.getActiveClients()).toHaveLength(1);
296+
expect(TestWebSocket.instances).toHaveLength(0);
297+
expect(testTransport.disposeCalls).toBe(0);
298+
299+
await vi.advanceTimersByTimeAsync(1);
300+
301+
expect(onClientIdle).toHaveBeenCalledOnce();
302+
expect(manager.getActiveClients()).toHaveLength(0);
303+
expect(testTransport.disposeCalls).toBe(1);
304+
});
305+
});

0 commit comments

Comments
 (0)