autozimu / LanguageClient-neovim

Language Server Protocol (LSP) support for vim and neovim.
MIT License
3.55k stars 272 forks source link

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

Open mqudsi opened 3 years ago

mqudsi commented 3 years ago

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

To Reproduce

Steps to reproduce the behavior:

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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