clojure-lsp processes left running/orphaned if VS Code is closed while the lsp server is starting

[bpringe]

I need to investigate this more, but I noticed my system's memory was high and when I checked my processes there were three java processes, all clojure-lsp.jar, using a good bit of memory.

[bpringe]

It seems that if VS Code is closed while clojure-lsp is starting, the lsp process is not killed. If VS Code is closed after clojure-lsp has started, the lsp process is killed.

[bpringe]

Somehow this wasn't auto-closed. It's been released in 2.0.146.

[bpringe]

This seems to be an issue again in 2.0.148, though I couldn't reproduce after fixing before 🤔. Not sure if it's a regression or just wasn't fixed correctly before. The changes that I thought fixed it are in the current release.

If I can't get this fixed from Calva's end, it may need to be fixed in clojure-lsp, such as ending the process when the client disconnects.

[bpringe]

@ericdallo I noticed in the logs when the clojure-lsp is left running even after Calva is shut down, it shows "Shutdown" but not "Exit" -- the exit server function is not being called. Any idea what could be going on?

This only happens if VS Code is exited right when the clojure-lsp server begins to start. I think we've done about everything we can on Calva's side to tell the server to close. I even tried to explicitly send the exit request before calling client.stop() here.

I've referenced @borkdude's clj-kondo extension and he does the same things as us to ensure the server is shut down correctly, and the kondo server always seems to be killed when it should.

I wonder if something is going on with the clojure-lsp server that prevents it from exiting properly in this case (right at startup). The logs look like this when it doesn't shut down correctly (note it doesn't have an "Exit"):

INFO  clojure-lsp.main: Starting server...
INFO  clojure-lsp.main: ====== LSP nrepl server started on port 38909
INFO  clojure-lsp.main: Initializing...
INFO  clojure-lsp.crawler: Crawling dir /home/brandon/development/clojure-test/src
INFO  clojure-lsp.crawler: Crawling dir /home/brandon/development/clojure-test/test
INFO  clojure-lsp.crawler: Crawling dir other
INFO  clojure-lsp.crawler: Crawling dir src
INFO  clojure-lsp.crawler: skipping classpath scan due to project hash match
DEBUG clojure-lsp.main: :initialize 880 ()
INFO  clojure-lsp.main: Shutting down

What makes this a more typical scenario is that people will open one project and then quickly switch to another, like when VS Code automatically opens a project but they want another. In this case they have an orphanage clojure-lsp process now.

[bpringe]

Realizing now that exit is a notification. I'll see tomorrow if I can manually send the exit notification right after the shutdown request from Calva. I think the vscode-languageclient is supposed to do this in the stop function, but maybe the VS Code instance is being exited before the shutdown response is received, so it doesn't send the exit notification.

[ericdallo]

Yeah, exactly, never saw that behavior with emacs lsp-mode because even if server doesn't respond, it kill the process manually.
I can take a look tomorrow, check how other server handle this, but my guess is that the server is correct

[PEZ]

Not sure if this is at all relevant, but anyway. For the jack-in process we had to go through some special loops and hoops on Windows to kill it properly:

calva/src/nrepl/jack-in.ts

Line 80 in dd149f1

export function calvaJackout() {

[bpringe]

@PEZ Thanks. What's the purpose of the (Thread/sleep 5000) before shutdown-agents?

[PEZ]

I don't know. 😄 The solution is copied from clojure-emacs/cider#390 (comment) (and since it also fixed the orphan java processes we had on Windows, w/o causing any other troubles, we have let it be.)

[bpringe]

I've spent lots of time on this now and I'm running out of ideas from the client's perspective. My only ideas left are to see if we can heartbeat the client from the server, and have the server kill itself if the heartbeat fails.

Created this issue to see if there's anything else I can try, or if there's some bug upstream: microsoft/vscode-languageserver-node#726

[bpringe]

For the record, I think I managed to also orphan the clj-kondo lsp server once too, so I think this may indeed be a bug in vscode or the client library.

[bpringe]

Seems the only route left here after toying with things is modifying the server, unfortunately. I've verified again that the clj-kondo process does get orphaned sometimes too, as it seemed to happen multiple times today.

@ericdallo What would you suggest be done in the server? I can think of two things, one of which I'm not sure the best implementation:

1. We call System/exit in the shutdown method instead of exit (as well as shutdown-agents). This worked well in my tests, but I know it doesn't follow the protocol, and maybe makes some usages not work, like if shutdown could be used to restart the server without killing the process. I know this is probably not a good idea.
2. We add a heartbeat mechanism to the server. For example (just one idea), we return the launcher from run in clojure-lsp.main, and save it in an atom, then start continuous loop/process that checks if the client is still connected, like every few seconds, and if it's not, calls the server's exit.

I'm reluctant to push forward with making Calva rely more on clojure-lsp until this is fixed. Even though it's a small window of time in which this happens, it's right during a time window where people switch projects after opening VS Code, so I think it will be experienced fairly often, plus we've had a complaint already about it.

Another thought is if the server's performance is improved, this problem could potentially be eliminated, because it seems the processing at startup is what delays the shutdown call, which occurs so late after the request is made from the client, that the client process is killed before the server responds, it seems. (Happens faster if initialization is complete, which is why we don't see the problem then.)

Long rant! 😄

[ericdallo]

I see you really tested a lot the possibilities, but I can't see this as an issue on the server since we use this way for years for vim emacs and other editors, I'd prefer we don't touch on that code since we can add some regressions and it works well for other clients.

With the #906 (comment), I believe this is really an issue on VSCode side, right?

[ericdallo]

More info on how the protocol suggests that implementation here

[bpringe]

I understand your reasoning, and I've read the shutdown and exit parts of the protocol, which is why I said my # 1 didn't follow the protocol.

I have one more path to explore from the client side, and that's starting the process in detached mode, as mentioned here, which may give us more control over killing the child process (though they mention vscode not killing the server there, it's worth exploring).

Maybe I'll get some good response on the vscode issue I created 🤞.

[PEZ]

I can't reproduce the problem in development. Maybe VS Code is killing the dev extension host differently? Using a regular VS Code window the problem is immediate and obvious.

[bpringe]

Hmm.. I've reproduced consistently with the extension host, and verified I can in the dev branch too. I do find that it's consistent if killed immediately after the progress notification pops up. Could be an OS difference regarding the extension host though.

[bpringe]

@ericdallo I've got some additional info on this here. It seems that while there is a bug in VS Code, the server is also expected to monitor the client process and gracefully exit if the process no longer exists (it crashed). The processId param of the initializeRequest can be used for this. VS Code sends the processId in the request (not sure if other clients do).

I've briefly looked around at what methods are available from Java to check if a process with a given pid exists, and it seems the Java 9 Process API allows a check by pid, but the Java 8 one does not. So it seems a solution that works with Java 8 would require using shell commands, and using the right ones for the host OS.

I wonder your thoughts on a good solution to this. Clojure-lsp could also try to ping the client with a notification or something, but I think checking if the client process is still alive from the processId in the initializeRequest would be the canonical way.

[ericdallo]

Oh, nice I didn't see that on the LSP spec, good to know.
Yeah, I think the canonical way is to check the processId indeed, so maybe we need to check the old java8 way, or if have any Clojure way for that. I'd like to avoid using shell manually, but if we have no choice, we need to test it with Linux and Windows

[bpringe]

Is this something you want to work on or would you like me to?

[ericdallo]

Help is always welcome :)

[bpringe]

Fixed in clojure-lsp here

[bpringe]

Not sure if the auto close is delayed or didn't work, but this fix is released.