Fix for a race condition when starting language server #1780
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…) to complete This is a UT fixup for a change in _AwaitServerMessages. Note that ycm-core#1434 (comment) is not exactly correct together with ycm-core#1434. Quote: > (...) Note that the java completer has a ton of setup to do before it gets to the super().StartServer(). It's true that it has some additional tasks (wiping out workspace) before it calls *super()._StartServerNoLock()*. But PR 1434 doesn't address it - wiping workspace is called from JavaCompleter.StartServer() at which point _server_started is already True, so the following part from that PR in _AwaitServerMessages(): -return self._initialize_event.is_set() +return not self._server_started or self._initialize_event.is_set() did not make it return True. In other words, ycm-core#1434 seems to not address the JavaCompleter issue, but only the issue with potentially long extra conf Settings(). However according to ycm-core#1433 (comment) this should rather be solved by increasing MESSAGE_POLL_TIMEOUT. Anyway, one of the first things we do from OnFileReadyToParse() when server is not healthy is setting _server_started to True.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In the following scenario:
What happens from time to time is that LSP server (in my case clangd) started more than once. With more than one running clangd process, YouCompleteMe does not work at all, because it tries to use a server that is not initialized - initialize request was sent to another one. Workaround: quit vim, kill ycmd and clangd processes, start vim again.
The way I fixed it is that
_server_startedis only accessed within_server_info_mutexand only the first thread callsStartServer(). Note that it required to partially revert #1434. Given that_server_startedis now accessed only within the mutex, it does not make any sense for_AwaitServerMessagesto access it. For additional rationale about it, see 6b6084a commit message.This change is