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

LC continues to run if neovim crashes or is killed #1215

Open
mqudsi opened this issue Apr 8, 2021 · 4 comments
Open

LC continues to run if neovim crashes or is killed #1215

mqudsi opened this issue Apr 8, 2021 · 4 comments

Comments

@mqudsi
Copy link

mqudsi commented Apr 8, 2021

Describe the bug

If the language client is running and working perfectly, active for the current buffer, and neovim crashes or is killed (killall -9 nvim), languageclient and any processes it spawned continue to run indefinitely in the background.

Environment

  • neovim/vim version (nvim --version or vim --version): 0.5.0-dev
  • This plugin version (git rev-parse --short HEAD): a734a05
  • This plugin's binary version (bin/languageclient --version): 0.1.161
  • Minimal vimrc content (A minimal vimrc is the smallest vimrc that could
    reproduce the issue. Refer to an example here): n/a
  • Language server link and version: n/a (confirmed with rust-analyzer, but n/a)

To Reproduce

Steps to reproduce the behavior:

  • run neovim
  • start language client
  • in another shell session, kill neovim
  • check currently running processes or pidof languageclient to confirm it hasn't exited

Current behavior

LC does not detect that parent (n)vim session has ended and continues to run.

Expected behavior

LC should detect (neo)vim has quit and quit as well.

@martskins
Copy link
Collaborator

martskins commented Apr 8, 2021

I don't know if there's much we can do if you kill -9 neovim, the languageclient job is spawned by neovim, so if you kill (no -9) neovim it will kill it's child processes, including languageclient, but kill -9 doesn't necessarily do that, and I'm not sure if we can detect that somehow.
To be fair though, this is a pretty extreme case and falls very much out of the expected usage and users calling kill -9 should be aware that hanging child processes could be a thing. Happy to receive suggestions or PRs if you have any ideas though.

@mqudsi
Copy link
Author

mqudsi commented Apr 8, 2021

The kill -9 is just to model it, really. I have to kill neovim when LC hangs when its LSP hangs (or I can kill LC), but this also happens when neovim itself crashes (which is, I agree, not all that often).

Neovim's jobstart should create the child process with a pipe connected to the child's stdin, spawning a thread to read from the pipe and set the global abort flag when the read fails should be sufficient.

I tried to do this myself but I found that in src/main.rs there is already a reader from stdin? So it seems that perhaps the crash handler is not correctly triggering the abort?

@ulidtko
Copy link

ulidtko commented May 4, 2021

Cannot reproduce. killall nvim does terminate LC. The latter writes this line in its log (let g:LanguageClient_loggingFile = "/tmp/vim-lsp-client.log") before exiting:

02:51:45 INFO reader-None src/rpcclient.rs:241 reader-None terminated

Which speaks "working as expected" to me; the Vim RPC pipe reader does catch the broken pipe condition and terminates, taking LS with it as well.

@mqudsi are you sure you're observing the issue as described and not confusing something? Please recheck. Please also make sure to use the regular kill/killall/kill -TERM — not -9.

I also strongly agree with what @martskins has said: it's not reasonable to expect any specific behavior in response to kill -9 a.k.a. SIGKILL. There's literally nothing an application can do in response to it. SIGKILL is handled completely within the OS kernel.

The signals SIGKILL and SIGSTOP cannot be caught, blocked, or ignored.

@seqizz
Copy link

seqizz commented Aug 12, 2021

Just my 2 bits: You can detect if the parent has been changed or not (in case of kill -9 neovim, LC will have PID 1 as its parent). Related link: https://stackoverflow.com/a/12193625

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

No branches or pull requests

4 participants