[release] src/goLanguageServer: timeout LanguageClient.stop call
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/vscode-go#1896
For golang/vscode-go#2222
Change-Id: Idbcfd3ee5f94fd3fd8dcafa228c6f03f5e14b905
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/402834
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
Reviewed-by: Jamal Carvalho <jamal@golang.org>
(cherry picked from commit 9227019e7eca3691665e8c6a15b8cf332febdcf3)
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/403414
diff --git a/src/language/goLanguageServer.ts b/src/language/goLanguageServer.ts
index db21fcd..51f2903 100644
--- a/src/language/goLanguageServer.ts
+++ b/src/language/goLanguageServer.ts
@@ -358,19 +358,18 @@
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 @@
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;
