-
Notifications
You must be signed in to change notification settings - Fork 235
PSES needs to shutdown itself when it's supposed to #655
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Also I would recommend to PSES to shutdown itself when all communication channels are closed, e.g. when stdin pipe is closed and there is no way to talk with PSES anymore. |
Given that a single PSES instance will only ever talk to one client, but that that client can communicate with PSES over multiple channels (which I assume are all connection-oriented, guaranteed-delivery channels, so that we get sent an interrupt or a message when they are closed/broken), would the following behaviour make sense:
? |
@rjmholt would you please elaborate on the
Do we still consider it |
@yatli I don't quite follow your example. When you say the client ignores a channel, do you mean it doesn't try to connect to it at all? Or do you mean the service tries to connect to the client and the client ignores the request? I've done some reading on TCP channels, and realise I had a misconception of the guarantees provided by TCP (and I imagine pipes/stdio as well). (I thought the underlying protocol guaranteed knowledge of a broken connection, like it guarantees delivery). In which case we must send data over the channel with an expected response timeout to detect if it has broken. So a heartbeat. The problem there is that clients that don't implement a heartbeat response will just see PSES shutdown after a single timeout duration (I imagine ~5s). So... Perhaps we need yet another startup flag that defaults to "yes use a heartbeat" but which can also be set to "no, no heartbeat. I promise to shut down PSES myself". |
@rjmholt I mean the client ignores the channel and does not connect at all. |
Ok so good news... When the client disconnects, PSES throws an exception that the stdio stream has closed: So I thought this would be a great place to complete the following tasks that seem to be holding up the process from exiting: ... But it's still not shutting down... There has to be something else lingering... |
jk PR out 😄 |
Ok everyone, I did some research and here's what I've found:
Right now, the client kills the server process.
The proof can be found here in the VSCode client: https://github.com/PowerShell/vscode-powershell/blob/master/src/process.ts#L145-L161
That said, this is not a good thing.
You might be wondering... The LSP has both a
shutdown
and anexit
message, don't we implement those?The answer is yes.......... but it's functionally a no-op 😢
You can see the
TODO
comment here:https://github.com/PowerShell/PowerShellEditorServices/blob/master/src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs#L148
which is referring to this TaskCompletionSource that is causing the process to stay open:
PowerShellEditorServices/src/PowerShellEditorServices.Host/EditorServicesHost.cs
Line 341 in 4716b7f
All we need to do is pass that TaskCompletionSource into the handler and complete the source when an
exit
event is received (possibly when there are 0 connections)P.s. big thanks to @SeeminglyScience for helping me investigate this! 😄
The text was updated successfully, but these errors were encountered: