Let language server subprocess be its own process group #77
Conversation
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
In situations where `multilspy` is launched from another process, a SIGINT or SIGTERM signal will be received by both the launching process as well as the process running the language server because `asyncio.create_subprocess_shell` defaults to `start_new_session=False`. In such a situation, `lsp_protocol_handler.server.shutdown` will be called but will hang since the underlying process is already terminated, `lsp_protocol_handler.server` won't ever receive an answer back for `shutdown`. This change makes it so that SIGINT and SIGTERM won't propagate to the language server process. We rely on the clean up code already in place to shut down the process as desired. See [python documentation](https://docs.python.org/3.13/library/subprocess.html#subprocess.Popen) for more details on `start_new_session`
|
Nice catch! Thanks for the PR. Can we make this configurable? Just add a flag to MultilspyConfig, and set it to true by defualt. |
…yConfig` Default value is `True`, which means language server process will be in its own process group. SIGINT and SIGTERM will only be received by the `multilspy` python process itself, not propagated. We rely on the existing `shutdown` and `stop` code to clean up the process. If set to `False`, the language server process will be under the same process group as the `multilspy` python process. This means SIGINT and SIGTERM will be received by both processes. This may cause issues if the python process has clean up code that runs after receiving SIGINT/SIGTERM because the `start_server` context manager will begin running its cleanup code, part of which invokes the `shutdown` and `stop` in `language_protocol_handler.server`. This will hang forever while awaiting an response from the language server process, since it would have already terminated itself after receiving SIGINT/SIGTERM.
|
Definitely! Updated the PR to use |
LakshyAAAgrawal
approved these changes
Mar 8, 2025
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 situations where
multilspyis launched from another process, a SIGINT or SIGTERM signal will be received by both the launching process as well as the process running the language server becauseasyncio.create_subprocess_shelldefaults tostart_new_session=False. In such a situation,lsp_protocol_handler.server.shutdownwill be called but will hang since the underlying process is already terminated,lsp_protocol_handler.serverwon't ever receive an answer back forshutdown.This change makes it so that SIGINT and SIGTERM won't propagate to the language server process. We rely on the clean up code already in place to shut down the process as desired.
See python documentation for more details on
start_new_session