Skip to content

Commit

Permalink
[release] 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 #1896
For #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]>
(cherry picked from commit 9227019)
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/403414
  • Loading branch information
hyangah committed May 2, 2022
1 parent d76c22f commit 48c6b08
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
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down

0 comments on commit 48c6b08

Please sign in to comment.