Skip to content
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

[LSP] shutdown and exit commands throw an exception #1113

Closed
LoneBoco opened this issue Feb 15, 2018 · 7 comments
Closed

[LSP] shutdown and exit commands throw an exception #1113

LoneBoco opened this issue Feb 15, 2018 · 7 comments
Labels

Comments

@LoneBoco
Copy link
Contributor

LoneBoco commented Feb 15, 2018

I am trying to use omnisharp-roslyn in LSP mode and I am unable to shut down the server through normal means.

LSP:  --> shutdown
server: [dbug]: OmniSharp.Extensions.LanguageServer.LspRequestRouter
        Finding descriptor for shutdown
LSP:server: [crit]: OmniSharp.Extensions.LanguageServer.LspRequestRouter
        Failed to handle notification shutdown
System.Reflection.TargetParameterCountException: Parameter count mismatch.
   at System.Reflection.RuntimeMethodInfo.InvokeArgumentsCheck(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at OmniSharp.Extensions.JsonRpc.ReflectionRequestHandlers.HandleRequest(IHandlerDescriptor instance, CancellationToken token)
   at OmniSharp.Extensions.LanguageServer.LspRequestRouter.<RouteRequest>d__11.MoveNext()

I tried throwing /shutdownServer to see what would happen (it is currently listed as not supported), and I get this response:

LSP:  --> shutdownServer
server: [dbug]: OmniSharp.Extensions.LanguageServer.LspRequestRouter
        Finding descriptor for shutdownServer
server: [dbug]: OmniSharp.Extensions.LanguageServer.LspRequestRouter
        Unable to find shutdownServer, methods found include initialize:OmniSharp.Extensions.LanguageServer.LanguageServer, initialized:OmniSharp.Extensions.LanguageServer.LanguageServer, shutdown:OmniSharp.Extensions.LanguageServer.Handlers.ShutdownHandler, exit:OmniSharp.Extensions.LanguageServer.Handlers.ExitHandler, $/cancelRequest:OmniSharp.Extensions.LanguageServer.Handlers.CancelRequestHandler, textDocument/didChange:OmniSharp.LanguageServerProtocol.Handlers.TextDocumentSyncHandler, textDocument/didOpen:OmniSharp.LanguageServerProtocol.Handlers.TextDocumentSyncHandler, textDocument/didClose:OmniSharp.LanguageServerProtocol.Handlers.TextDocumentSyncHandler, textDocument/didSave:OmniSharp.LanguageServerProtocol.Handlers.TextDocumentSyncHandler, textDocument/willSave:OmniSharp.LanguageServerProtocol.Handlers.TextDocumentSyncHandler, textDocument/willSaveWaitUntil:OmniSharp.LanguageServerProtocol.Handlers.TextDocumentSyncHandler, textDocument/didChange:OmniSharp.LanguageServerProtocol.Handlers.TextDocumentSyncHandler, textDocument/didOpen:OmniSharp.LanguageServerProtocol.Handlers.TextDocumentSyncHandler, textDocument/didClose:OmniSharp.LanguageServerProtocol.Handlers.TextDocumentSyncHandler, textDocument/didSave:OmniSharp.LanguageServerProtocol.Handlers.TextDocumentSyncHandler, textDocument/willSave:OmniSharp.LanguageServerProtocol.Handlers.TextDocumentSyncHandler, textDocument/willSaveWaitUntil:OmniSharp.LanguageServerProtocol.Handlers.TextDocumentSyncHandler, textDocument/definition:OmniSharp.LanguageServerProtocol.Handlers.DefinitionHandler, textDocument/definition:OmniSharp.LanguageServerProtocol.Handlers.DefinitionHandler, textDocument/hover:OmniSharp.LanguageServerProtocol.Handlers.HoverHandler, textDocument/hover:OmniSharp.LanguageServerProtocol.Handlers.HoverHandler, textDocument/completion:OmniSharp.LanguageServerProtocol.Handlers.CompletionHandler, textDocument/completion:OmniSharp.LanguageServerProtocol.Handlers.CompletionHandler, textDocument/signatureHelp:OmniSharp.LanguageServerProtocol.Handlers.SignatureHelpHandler, textDocument/signatureHelp:OmniSharp.LanguageServerProtocol.Handlers.SignatureHelpHandler, textDocument/rename:OmniSharp.LanguageServerProtocol.Handlers.RenameHandler, textDocument/documentSymbol:OmniSharp.LanguageServerProtocol.Handlers.DocumentSymbolHandler, textDocument/documentSymbol:OmniSharp.LanguageServerProtocol.Handlers.DocumentSymbolHandler

shutdown and exit seem to be handled somewhere else. I tried briefly searching through the code of omnisharp-roslyn, but I couldn't find out where it was handled.

shutdown:OmniSharp.Extensions.LanguageServer.Handlers.ShutdownHandler
exit:OmniSharp.Extensions.LanguageServer.Handlers.ExitHandler

EDIT: I should mention that I am running Omnisharp with the -lsp and -stdio command line arguments.

@david-driscoll
Copy link
Member

This should be fixed in the next version of the lsp library (that we have not yet updated to).

@mholo65 is working on a code lens handler, and the upgrade will roll in with that.

@LoneBoco
Copy link
Contributor Author

LoneBoco commented Feb 16, 2018

Thanks for that hint. I was able to figure out how it works and merge the LSP library version 0.7.7 into my own code. I have discovered a bunch of other problems, but I don't know where they should be reported. I guess in csharp-language-server-protocol? One issue is that OmniSharp.exe doesn't terminate when it receives the exit request (although it doesn't throw an exception anymore). The other is that it tells the client that it has no capabilities and I'm not sure if that is a problem with this project, the other one, or if that just never got implemented yet.

@LoneBoco
Copy link
Contributor Author

LoneBoco commented Feb 16, 2018

Well, the bug with OmniSharp.exe not terminating is because @mholo65 's current code doesn't bind the server's Exit event in LanguageServerHost.cs. Since you said it was a work in progress, I'll just leave it alone for now.

For the other one, I will make the issue here.

@david-driscoll
Copy link
Member

@LoneBoco the language server library actually manages exit and shutdown for you @ https://github.com/OmniSharp/csharp-language-server-protocol/blob/master/src/Server/LanguageServer.cs#L88

@david-driscoll
Copy link
Member

However we did have an issue with shutdown and exit not working correctly, that has been fixed in the library, it just hasn't been merged yet.

@LoneBoco
Copy link
Contributor Author

Is that so? Does the library forcibly kill the process when that happens?

According to this, Omnisharp waits on the cancellation token when we start it up:

cancellation.Token.WaitHandle.WaitOne();

And here we can see that we don't send the cancellation token unless we abort in the console, or unless the host process terminates:

Console.CancelKeyPress += (sender, e) =>
{
_cancellationTokenSource.Cancel();
e.Cancel = true;
};
if (_environment.HostProcessId != -1)
{
try
{
var hostProcess = Process.GetProcessById(_environment.HostProcessId);
hostProcess.EnableRaisingEvents = true;
hostProcess.OnExit(() => _cancellationTokenSource.Cancel());
}
catch
{
// If the process dies before we get here then request shutdown
// immediately
_cancellationTokenSource.Cancel();
}
}

There is nothing that actually triggers that cancellation token if we receive an exit command.

For what it is worth, I added a simple block of code and it now exits for me:

_server.Exit += (sender) =>
{
    _cancellationTokenSource.Cancel();
};

This is because the library sends out that Exit event when it receives the exit command.

@bjorkstromm
Copy link
Member

bjorkstromm commented Jan 26, 2019

Fixed in 1.32.10 through #1345

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants