ebkalderon / tower-lsp

Language Server Protocol implementation written in Rust
Apache License 2.0
962 stars 54 forks source link

Automatic exit #328

Closed Timmmm closed 2 years ago

Timmmm commented 2 years ago

I was a bit surprised to find a load of my tower-lsp language server processes still running long after VSCode had quit. I had assumed that VSCode's LanguageClient would automatically kill them, or maybe tower-lsp would handle it, but that does not seem to be the case.

Deno has a "solution" that basically polls if the parent process still exist and calls exit() if not. Seems ugly but workable. But I wonder if tower-lsp should take care of that for me?

ebkalderon commented 2 years ago

That does seem odd! According to the LSP spec, the server process must receive an exit notification from the client in order to quit. I'm not aware of any issues with the exit notification handler in tower-lsp as of release 0.16.0, which correctly disconnects the Client loopback channel immediately when exit is received and/or the stdin pipe is closed upstream. Exits should occur automatically already, assuming that the notification is sent and received by the server.

Do we know for sure whether something is indeed broken with tower-lsp's implementation of exit, or if perhaps VSCode isn't sending the exit notification to servers in certain instances?

Timmmm commented 2 years ago

Hmm interesting. That's weird. Is it possible that VSCode crashes and doesn't send the exit notification and somehow leaves stdin open?

I will do some experimentation.

ebkalderon commented 2 years ago

Sounds good! Please feel free to post here whatever you find. I'd be happy to accept pull requests against tower-lsp if a bug in the exit notification handler is found.

Timmmm commented 2 years ago

Hmm so I tried just running my server under Bash and then killing Bash but that did kill the server. Maybe LanguageClient starts it in a different way. I'll try that next.

You can see that I definitely have a few server instances still running that have been detached from VSCode:

$ ps -u me --forest
   PID TTY          TIME CMD
247585 ?        00:00:00 sshd
247586 ?        00:00:00  \_ qrsh_starter
247765 ?        00:00:00      \_ bash
247985 ?        00:00:00          \_ sshd
248025 ?        00:00:00              \_ sshd
248026 ?        00:00:00                  \_ bash
248209 ?        00:00:00                  |   \_ bash
248306 ?        00:00:00                  |       \_ sh
248319 ?        00:00:04                  |       |   \_ node
248487 ?        00:00:01                  |       |       \_ node
250008 pts/62   00:00:00                  |       |       |   \_ bash
252112 pts/57   00:00:00                  |       |       |   \_ bash
252646 pts/57   00:00:00                  |       |       |       \_ ps
249430 ?        00:00:04                  |       |       \_ node
249634 ?        00:00:19                  |       |       |   \_ cpptools
249945 ?        00:00:00                  |       |       |   |   \_ cpptools-srv
249779 ?        00:00:00                  |       |       |   \_ server_linux  <-- This is fine
249441 ?        00:00:01                  |       |       \_ node
250513 ?        00:00:07                  |       |       \_ node
250679 ?        00:00:18                  |       |       |   \_ cpptools
250771 ?        00:00:00                  |       |       |   \_ server_linux  <-- This is fine
250835 ?        00:00:00                  |       |       |   \_ node
250858 ?        00:00:00                  |       |       |   \_ lemminx-linux
250868 ?        00:00:25                  |       |       |   \_ node
250526 ?        00:00:07                  |       |       \_ node
248482 ?        00:00:00                  |       \_ sleep
248563 ?        00:00:00                  \_ bash
248746 ?        00:00:00                  |   \_ bash
248747 ?        00:00:00                  |       \_ sleep
248748 ?        00:00:00                  \_ bash
248856 ?        00:00:00                  |   \_ bash
248904 ?        00:00:00                  |       \_ sleep
248905 ?        00:00:00                  \_ bash
249019 ?        00:00:00                  |   \_ bash
249020 ?        00:00:00                  |       \_ sleep
249154 ?        00:00:00                  \_ bash
249264 ?        00:00:00                  |   \_ bash
249316 ?        00:00:00                  |       \_ sleep
249320 ?        00:00:00                  \_ bash
249427 ?        00:00:00                  |   \_ bash
249428 ?        00:00:00                  |       \_ sleep
250200 ?        00:00:00                  \_ bash
250316 ?        00:00:00                  |   \_ bash
250368 ?        00:00:00                  |       \_ sleep
250373 ?        00:00:00                  \_ bash
250507 ?        00:00:00                      \_ bash
250508 ?        00:00:00                          \_ sleep
163406 ?        00:00:00 sh
163416 ?        00:03:06  \_ node
163454 ?        00:00:51      \_ node
196144 ?        00:00:00 server_linux  <-- Where is node?
262277 ?        00:00:00 sh
262287 ?        00:03:30  \_ node
262398 ?        00:01:01      \_ node
187619 ?        00:00:00 server_linux  <-- Ditto
186865 ?        00:00:00 server_linux  <-- Ditto

(Yeah don't ask about the sshd/bash insanity!)

Timmmm commented 2 years ago

Aha so it does seem that killing the node parent just detaches the server:

 ~ ps -u timh --forest
   PID TTY          TIME CMD
...
 38934 ?        00:00:01                  |       |   \_ node
 38992 ?        00:00:00                  |       |       \_ node
 39993 pts/164  00:00:00                  |       |       |   \_ bash
 39326 ?        00:00:08                  |       |       \_ node              <- kill this
 39642 ?        00:00:19                  |       |       |   \_ cpptools
 44658 ?        00:00:00                  |       |       |   |   \_ cpptools-srv
 39863 ?        00:00:00                  |       |       |   \_ server_linux  <- and this is detached
 51093 ?        00:00:01                  |       |       |   \_ git
 39357 ?        00:00:02                  |       |       \_ node
 51060 ?        00:00:00                  |       \_ sleep
...
 ~ kill -9 39326
 ~ ps -u timh --forest
   PID TTY          TIME CMD
...
 38934 ?        00:00:01                  |       |   \_ node
 38992 ?        00:00:00                  |       |       \_ node
 39993 pts/164  00:00:00                  |       |           \_ bash
 51060 ?        00:00:00                  |       \_ sleep
...
 39863 ?        00:00:00 server_linux
...

Interestingly the cpptools and git processes do die. Here's how I start the server:

    let serverOptions: ServerOptions = {
        run: {
            command: serverCommand,
            args: ["--ipc-path", ipcPath],
        },
        debug: {
            command: serverCommand,
            args: ["--ipc-path", ipcPath, "--debug"],
        },
    };

    // Options to control the language client
    let clientOptions: LanguageClientOptions = {
        // Register the server for ipcfg documents.
        documentSelector: [{ scheme: "file", language: "ipcfg" }],
    };

    // Create the language client and start the client.
    client = new LanguageClient(
        "ipcfg",
        "My extension",
        serverOptions,
        clientOptions
    );

Nothing special. I guess the next thing to try is commenting out code in my server. The only thing I can think it would be is that I tokio::spawn() a task and never make any effort to stop it. Would that block the server from exiting?

Timmmm commented 2 years ago

Hmm the Tokio docs say:

When a runtime is shutdown, all outstanding tasks are dropped, regardless of the lifecycle of that task.

So I don't think my tokio::spawn() is the issue. I got a backtrace from one of the detached processes, but it doesn't really show much except that Tokio is waiting for something.

#0  0x00002ad18ffcca35 in pthread_cond_wait@@GLIBC_2.3.2 () from /lib64/libpthread.so.0
#1  0x000055ee91696b78 in tokio::park::thread::Inner::park::hfc2ab6e138c4e564 ()
#2  0x000055ee91697279 in <tokio::park::thread::CachedParkThread as tokio::park::Park>::park ()
#3  0x000055ee9151d8da in tokio::runtime::enter::Enter::block_on ()
#4  0x000055ee914d4d52 in tokio::runtime::Runtime::block_on ()
#5  0x000055ee914f694a in server::main ()
#6  0x000055ee91531b93 in std::sys_common::backtrace::__rust_begin_short_backtrace ()
#7  0x000055ee915491d9 in std::rt::lang_start::_$u7b$$u7b$closure$u7d$$u7d$::h5575c1d24fb79702 ()
#8  0x000055ee916ca41a in core::ops::function::impls::{{impl}}::call_once<(),Fn<()>> ()
    at /rustc/9bc8c42bb2f19e745a63f3445f1ac248fb015e53/library/core/src/ops/function.rs:259
#9  std::panicking::try::do_call<&Fn<()>,i32> () at library/std/src/panicking.rs:379
#10 std::panicking::try<i32,&Fn<()>> () at library/std/src/panicking.rs:343
#11 std::panic::catch_unwind<&Fn<()>,i32> () at library/std/src/panic.rs:431
#12 std::rt::lang_start_internal () at library/std/src/rt.rs:51
#13 0x000055ee914f6a32 in main ()

I'll try commenting out code or maybe Tokio Console.

Timmmm commented 2 years ago

Hmm ok well I updated all my dependencies including tower-lsp, and I changed my buildsystem a little. Now I can't reproduce it. 🤷