natebosch / vim-lsc

A vim plugin for communicating with a language server
BSD 3-Clause "New" or "Revised" License
692 stars 83 forks source link

Error messages on exit... #201

Closed rsdubtso closed 5 years ago

rsdubtso commented 5 years ago

Hi,

I'm observing the following error messages each time I exit neovim:

[lsc:Error] Error dispatching message: 'Vim(call):E716: Key not present in Dictionary: _channel.notify(''exit'', v:null) " Don''t block on server status'
[lsc:Error] Command exited unexpectedly: cpp

I'm not sure if this is an issue with vim-lsc, and I want a second opinion :)

In my setup there are two language servers: clangd for cpp and pyls for python. I spent some time trying to debug this, and the exception is thrown when exiting from an editing session in which I only worked on cpp files and when vim-lsc is trying to send a request to the python language server. The following patch seems to fix things, although I'm not sure why

diff --git a/autoload/lsc/server.vim b/autoload/lsc/server.vim
index 8c7cc3c..950dc15 100644
--- a/autoload/lsc/server.vim
+++ b/autoload/lsc/server.vim
@@ -75,12 +75,15 @@ endfunction
 " Calls `OnExit` after the exit is requested. Returns `v:false` if no request
 " was made because the server is not currently running.
 function! s:Kill(server, status, OnExit) abort
+  if a:server.status != 'running'
+    return v:false
+  endif
   function! Exit(result) closure abort
     let a:server.status = a:status
     call a:server._channel.notify('exit', v:null) " Don't block on server status
     if a:OnExit != v:null | call a:OnExit() | endif
   endfunction
-  call a:server.request('shutdown', v:null, function('Exit'))
+  return a:server.request('shutdown', v:null, function('Exit'))
 endfunction

 function! lsc#server#restart() abort

The bottom change is there to make sure that s:Kill actually returns a status, but the top change actually duplicates the check for status == 'running'in the server.request() function. The issue does not happen with vim, so it may be a neovim bug.

Some additional info


vim-lsc: ad9d2bd0582419e902bc402545da397634202ed0

relevant portion of my vimrc:

let g:lsc_server_commands = {
            \ 'python': {
            \   'name': 'python',
            \   'command': 'pyls',
            \   'enabled': v:true,
            \   'suppress_stderr': v:true,
            \ },
            \ 'cpp' : {
            \   'name': 'cpp',
            \   'command': 'clangd',
            \   'enabled': v:true,
            \   'suppress_stderr': v:true,
            \ },
            \ }

neovim version (this is from Ubuntu 19.04):

NVIM v0.3.4
Build type: Release
LuaJIT 2.1.0-beta3
Compilation: /usr/bin/cc -g -O2 -fdebug-prefix-map=/build/neovim-tWk8pe/neovim-0.3.4=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=1 -DDISABLE_LOG -Wdate-time -D_FORTIFY_SOURCE=1 -Wconversion -O2 -DNDEBUG -DMIN_LOG_LEVEL=3 -Wall -Wextra -pedantic -Wno-unused-parameter -Wstrict-prototypes -std=gnu99 -Wimplicit-fallthrough -Wvla -fstack-protector-strong -fdiagnostics-color=auto -Wno-array-bounds -DINCLUDE_GENERATED_DECLARATIONS -D_GNU_SOURCE -DNVIM_MSGPACK_HAS_FLOAT32 -DNVIM_UNIBI_HAS_VAR_FROM -I/build/neovim-tWk8pe/neovim-0.3.4/build/config -I/build/neovim-tWk8pe/neovim-0.3.4/src -I/usr/include -I/build/neovim-tWk8pe/neovim-0.3.4/build/src/nvim/auto -I/build/neovim-tWk8pe/neovim-0.3.4/build/include
Compiled by team+vim@tracker.debian.org

Features: +acl +iconv +jemalloc +tui
See ":help feature-compile"

   system vimrc file: "$VIM/sysinit.vim"
  fall-back for $VIM: "/usr/share/nvim"

Run :checkhealth for more info
natebosch commented 5 years ago

The protocol defines 2 calls to exit, if the server is exiting after the first call and not waiting for the second then maybe we get into a weird state...

The first added check for server.status feels like it shouldn't be necessary. The switch from call to return is a clear bug fix - thanks!

I think the most direct workaround is to check for the channel's existence directly rather than worrying about status on the final 'exit' notification. I've got a change that I'm hoping fixes this - if it doesn't let me know and we can investigate further.

rsdubtso commented 5 years ago

I'm still getting

[lsc:Error] Command exited unexpectedly: cpp

after updating to 329d894f941945287e9797c42e6125838c8a2ac0. I'll try to post more info as I'm debugging this.

rsdubtso commented 5 years ago

I'm not reopening this since while I now know what happens, I am not sure still that the issue lies within vim-lsc and not with neovim. More precisely, I am now suspecting that the latter is more likely...

The problem is that, in neovim, when the Exit() capture is being called, it is called with the last value of the l:server variable in the lsc#server#exit() function and not the one with which it was initially captured.

On other words, since I have two servers, clangd and pyls (in this order), the s:Kill() is first invoked for clangd and then for pyls. The first call succeeds and registers the Exit() callback and the second fails. However, when the Exit() callback actually executes its a:server actually points to the pyls server.

It is as if the capture holds the reference to the variable and not to its value. This way the Exit() callback tries to kill my second pyls server, which is not active, and not the clang server which is it was intended to kill. The clangd server is never notified of the shutdown, hence the error message about unexpected exit.

This does not happen in vim where the a:server points to expected clangd record.

Hope this makes sense...

natebosch commented 5 years ago

Fixed in #201

The problem was I was using function and as you discovered I should have been using funcref - the latter points by reference, whereas function points by name which was getting overwritten when there are multiple servers.