ruff server no longer hangs after shutdown
#11222
Merged
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
snowsignal
force-pushed
the
jane/server/shutdown
branch
from
7d0fba9
to
cffeb09
Compare
|
MichaReiser
approved these changes
May 1, 2024
snowsignal
force-pushed
the
jane/server/shutdown
branch
from
3ca30b7
to
7739d0f
Compare
snowsignal
force-pushed
the
jane/server/shutdown
branch
from
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.
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.
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.