PSES needs to shutdown itself when it's supposed to

[TylerLeonhardt]

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 an exitmessage, don't we implement those?

The answer is yes.......... but it's functionally a no-op 😢

You can see the TODOcomment 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

this.serverCompletedTask = new TaskCompletionSource<bool>();

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! 😄

[nanoant]

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.
Still sending exit would be nice graceful way for clients to issue the termination request.

[rjmholt]

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:

• PSES keeps a list of all open channels, when they all close it shuts down.
• If the client sends an exit message on any (or the relevant) channel, PSES closes all channels and shuts down.
• If a single channel is closed but others remain open PSES keeps going.
• If a single channel fails but others remain open PSES keeps going, and it's up to the client to reopen the channel.

?

[yatli]

@rjmholt would you please elaborate on the open part? consider the whole lifecycle as follows:

PSES starts --> PSES setting up channel listeners ---> PSES waiting for incoming connection ---> The client ignores this channel, keep listening
                                    `
                                     `---------> PSES directly accepts stdio, but that's only a single channel ----> Client showing up here, do this and that, and then exits. Channel broken. ---> [ ? ]

Do we still consider it open?

[rjmholt]

@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".

[yatli]

@rjmholt I mean the client ignores the channel and does not connect at all.

[TylerLeonhardt]

Ok so good news...

When the client disconnects, PSES throws an exception that the stdio stream has closed:

https://github.com/PowerShell/PowerShellEditorServices/blob/master/src/PowerShellEditorServices.Protocol/MessageProtocol/ProtocolEndpoint.cs#L343

So I thought this would be a great place to complete the following tasks that seem to be holding up the process from exiting:
https://github.com/PowerShell/PowerShellEditorServices/blob/master/src/PowerShellEditorServices.Protocol/MessageProtocol/ProtocolEndpoint.cs#L100
And
https://github.com/PowerShell/PowerShellEditorServices/blob/master/src/PowerShellEditorServices.Host/EditorServicesHost.cs#L352

... But it's still not shutting down... There has to be something else lingering...

[TylerLeonhardt]

jk PR out 😄

#663