Skip to content
This repository was archived by the owner on May 30, 2024. It is now read-only.

Make sure we stop the current client before restarting#369

Merged
vinistock merged 1 commit intomainfrom
vs/fix_hanging_processes
Feb 3, 2023
Merged

Make sure we stop the current client before restarting#369
vinistock merged 1 commit intomainfrom
vs/fix_hanging_processes

Conversation

@vinistock
Copy link
Copy Markdown
Member

@vinistock vinistock commented Feb 2, 2023

Closes Shopify/ruby-lsp#1505

Given this scenario:

  1. User starts the Ruby LSP on a project that needs bundle install
  2. Instead of clicking on bundle install, run bundle install through the terminal
  3. Change Gemfile, Gemfile.lock or any other watched files
  4. This would trigger a restart and spawn a server process (which would work because gems are now installed)
  5. After this process has been spawned, click bundle install on the dialogue
  6. That would spawn a second LSP client and two processes would run

The reason this happens is because

  1. We weren't awaiting the call to restart
  2. While the dialogue was waiting for an answer, we hadn't set this.client. That means that invoking stop didn't create a stop promise, because the client wasn't set yet

The fix is to properly await the promises and set this.client early, so that we can properly return a stop promise when restarting a client that is hanging waiting for a dialogue confirmation.

@vinistock vinistock added the bugfix This PR will fix an existing bug label Feb 2, 2023
@vinistock vinistock self-assigned this Feb 2, 2023
@vinistock vinistock requested a review from a team as a code owner February 2, 2023 21:07
@vinistock vinistock merged commit 3dc9bea into main Feb 3, 2023
@vinistock vinistock deleted the vs/fix_hanging_processes branch February 3, 2023 14:31
jayanth-kumar-morem pushed a commit to jayanth-kumar-morem/vscode-ruby-lsp that referenced this pull request Apr 26, 2023
…p-minitest-0.24.0

Bump rubocop-minitest from 0.23.2 to 0.24.0
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bugfix This PR will fix an existing bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

After some usage I see several LSP instances running

3 participants