Skip to content

Commit

Permalink
src/goLanguageServer: timeout LanguageClient.stop call
Browse files Browse the repository at this point in the history
LanguageClient.stop hangs indefinitely if the language server
fails to respond to the `shutdown` request. For example, in
go.dev/issues/52543 we observed `gopls` crashes during
shutdown.

Implement a timeout from our side. (2sec)

Caveat:
If gopls is still active but fails to respond within 2sec,
it's possible that we may end up having multiple
gopls instances briefly until the previous gopls completes
the shutdown process.

For golang#1896
For golang#2222

Change-Id: Idbcfd3ee5f94fd3fd8dcafa228c6f03f5e14b905
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/402834
Run-TryBot: Hyang-Ah Hana Kim <[email protected]>
TryBot-Result: kokoro <[email protected]>
Reviewed-by: Suzy Mueller <[email protected]>
Reviewed-by: Jamal Carvalho <[email protected]>
hyangah committed Apr 29, 2022
1 parent 27e8258 commit 9227019
Showing 1 changed file with 35 additions and 7 deletions.
42 changes: 35 additions & 7 deletions src/language/goLanguageServer.ts
Original file line number Diff line number Diff line change
@@ -358,19 +358,18 @@ export const flushGoplsOptOutConfig = (cfg: GoplsOptOutConfig, workspace: boolea
async function startLanguageServer(ctx: vscode.ExtensionContext, config: LanguageServerConfig): Promise<boolean> {
// If the client has already been started, make sure to clear existing
// diagnostics and stop it.
let cleanStop = true;
if (languageClient) {
if (languageClient.diagnostics) {
languageClient.diagnostics.clear();
}
await languageClient.stop();
cleanStop = await stopLanguageClient(languageClient);
if (languageServerDisposable) {
languageServerDisposable.dispose();
}
}

// Check if we should recreate the language client. This may be necessary
// if the user has changed settings in their config.
if (!deepEqual(latestConfig, config)) {
// Check if we should recreate the language client.
// This may be necessary if the user has changed settings
// in their config, or previous session wasn't stopped cleanly.
if (!cleanStop || !deepEqual(latestConfig, config)) {
// Track the latest config used to start the language server,
// and rebuild the language client.
latestConfig = config;
@@ -411,6 +410,35 @@ async function startLanguageServer(ctx: vscode.ExtensionContext, config: Languag
return true;
}

const race = function (promise: Promise<unknown>, timeoutInMilliseconds: number) {
let token: NodeJS.Timeout;
const timeout = new Promise((resolve, reject) => {
token = setTimeout(() => reject('timeout'), timeoutInMilliseconds);
});
return Promise.race([promise, timeout]).then(() => clearTimeout(token));
};

// exported for testing.
export async function stopLanguageClient(c: LanguageClient): Promise<boolean> {
if (!c) return false;

if (c.diagnostics) {
c.diagnostics.clear();
}
// LanguageClient.stop may hang if the language server
// crashes during shutdown before responding to the
// shutdown request. Enforce client-side timeout.
// TODO(hyangah): replace with the new LSP client API that supports timeout
// and remove this.
try {
await race(c.stop(), 2000);
} catch (e) {
c.outputChannel?.appendLine(`Failed to stop client: ${e}`);
return false;
}
return true;
}

function toServerInfo(res?: InitializeResult): ServerInfo | undefined {
if (!res) return undefined;

0 comments on commit 9227019

Please sign in to comment.