ruff server no longer hangs after shutdown#11222
Merged
snowsignal merged 3 commits intomainfrom May 3, 2024
Merged
Conversation
7d0fba9 to
cffeb09
Compare
Contributor
|
MichaReiser
approved these changes
May 1, 2024
3ca30b7 to
7739d0f
Compare
7739d0f to
1e09b46
Compare
snowsignal
added a commit
that referenced
this pull request
May 5, 2024
## Summary A follow-up to #11222. `ruff server` stalls during shutdown with Neovim because after it receives an exit notification and closes the I/O thread, it attempts to log a success message to `stderr`. Removing this log statement fixes this issue. ## Test Plan Track the instances of `ruff` in the OS task manager as you open and close Neovim. A new instance should appear when Neovim starts and it should disappear once Neovim is closed.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
Fixes #11207.
The server would hang after handling a shutdown request on
IoThreads::join()because a global sender (MESSENGER, used to sendwindow/showMessagenotifications) would remain allocated even after the event loop finished, which kept the writer I/O thread channel open.To fix this, I've made a few structural changes to
ruff server. I've wrapped the send/receive channels and thread join handle behind a new struct,Connection, which facilitates message sending and receiving, and also runsIoThreads::join()after the event loop finishes. To control the number of sender channels, theConnectionwraps the sender channel in anArcand only allows the creation of a wrapper type,ClientSender, which hold a weak reference to thisArcinstead of direct channel access. The wrapper type implements the channel methods directly to prevent access to the inner channel (which would allow the channel to be cloned). ClientSender's function is analogous toWeakSenderintokio. Additionally, the receiver channel cannot be accessed directly - theConnectiononly exposes an iterator over it.These changes will guarantee that all channels are closed before the I/O threads are joined.
Test Plan
Repeatedly open and close an editor utilizing
ruff serverwhile observing the task monitor. The net total amount of openruffinstances should be zero once all editor windows have closed.The following logs should also appear after the server is shut down:
This can be tested on VS Code by changing the settings and then checking
Output.