Repository navigation
client.needsStop() and client.stop() disagree on whether it's valid to call stop() #1559
Description
I'm trying to fix some issues with the LSP client showing errors when I try to dispose it. One issue is that I'm getting errors like this:
rejected promise not handled within 1 second: Error: Client is not running and can't be stopped. It's current state is: starting
extensionHostProcess.js:155
stack trace: Error: Client is not running and can't be stopped. It's current state is: starting
at LanguageClient.shutdown (d:\Dev\Dart-Code\Dart-Code\out\dist\extension.js:38362:10)
at LanguageClient.stop (d:\Dev\Dart-Code\Dart-Code\out\dist\extension.js:38332:15)
at LanguageClient.stop (d:\Dev\Dart-Code\Dart-Code\out\dist\extension.js:47462:22)
at Object.dispose (d:\Dev\Dart-Code\Dart-Code\out\dist\extension.js:4810:66)
at disposeAll (d:\Dev\Dart-Code\Dart-Code\out\dist\extension.js:29122:20)
at LspAnalyzer.dispose (d:\Dev\Dart-Code\Dart-Code\out\dist\extension.js:25645:32)
at d:\Dev\Dart-Code\Dart-Code\out\dist\extension.js:14511:36
at tryCleanup (d:\Dev\Dart-Code\Dart-Code\out\dist\extension.js:14544:15)
The problem is that calling stop() is invalid when the state is stopping, however my code looks like this:
if (this.client.needsStop())
await this.client.stop();The implementation of needsStop is:
vscode-languageserver-node/client/src/common/client.ts
Lines 1228 to 1230 in f58f4df
So needsStop says that stop is required if the state is starting, but calling stop() throws. It's not clear to me what the correct way of shutting down the server cleanly is.
Activity
[-]clietn.needsStop() and client.stop() disagree on whether it's valid to call stop()[/-][+]client.needsStop() and client.stop() disagree on whether it's valid to call stop()[/+]dbaeumer commentedon Oct 1, 2024
How do you start the server and which transport are you using? Looking through the code I think we need to allow clients to pass in a terminate function into the client#restart() method in case it is not a process started by the client itself.
dbaeumer commentedon Oct 1, 2024
See also #725 (comment)
DanTup commentedon Oct 1, 2024
I didn't realise there was a
restartmethod, but I think this applies to juststop()too. For some cases where I restart the server, things like the active Dart SDK might be changing so I basically restart the entire extension (basically calldeactivate()and thenactivate()internally). I think it should be possible to also do this cleanly (eg. not show the user errors if the server didn't shut down cleanly, and all registered commands must be unregistered so the new server can register them).dbaeumer commentedon Oct 2, 2024
Side note: have you checked that calling
deactivate()andactivate()on yourself is something that is supported from a VS Code extension side.DanTup commentedon Oct 2, 2024
They're just functions, so I'm not sure why they wouldn't (I just call them directly). I implemented this many years ago and it hasn't had any issues (except some edge cases for LSP noted here and #725).. It was the simplest way to ensure when something fundamental (like the SDK path) changes, we dispose everything old and create new versions without just implementing our own parallel version of disposing and recreating everything.
TheAngryByrd commentedon Oct 18, 2024
I think I'm running into something similar with the F# Ionide plugin. When attempting to restart, I'm getting the error.
Client is not running and can't be stopped. It's current state: running. This happens when the server process (a dotnet process) get terminated externally. This could happen by doingtaskkill/pkillbut a common one is updating the sdk on windows can terminate dotnet processes.