Sometimes when shutting down, Atom won't shutdown properly (some panes disappear but the window stays open) and the language server isn't stopped

[ghost]

Sometimes when shutting down, Atom Beta won't shutdown properly: instead of the Atom window disappearing, just all of the panes other than the main editor area disappear but the window stays open like this:

When I click the close button a second time Atom finally disappears, but the language server process keeps running in the background. Due to this odd effect, I am filing this here wondering if the atom-languageclient shutdown has potential side effects that can cause both the Atom shutdown to break and the language server not to get stopped.

[damieng]

This occurs when the language server does not respond to the shutdown message in a timely manner.

We have seen this with the ide-typescript package because the Sourcegraph javascript-typescript-languageserver does not respond to shutdown messages until it has finished analyzing the source for diagnostics. It's being tracked at sourcegraph/javascript-typescript-langserver#367

I'd like to avoid putting code in to explicitly time-out or otherwise terminate the language servers for now as that could cause their databases to get corrupt but if we start seeing this in a lot of packages it might be our only choice.

[ghost]

Yea my current experimental language server will also not respond during analysis and it can sometimes get stuck for quite a long time, so this is probably the same underlying problem.

Do all language servers you encountered use databases? Mine currently doesn't and it's therefore absolutely safe to kill at any time. However, I suppose if this is caused by language server hangs I should probably figure out a way on my own on how to respond during analysis and abort somehow.

Edit: I did forget, while mine wouldn't corrupt databases it would actually litter temporary files in /tmp - so you're right, this sounds like something the language server itself should handle better. But maybe at least some sort of pop up informing the user what is going on would be nice? (that the language server is misbehaving etc.)

[damieng]

One way of solving this is to ensure the work your language server does it broken into small chunks and placed in a queue. Between each piece of work you check incoming messages and queue them - potentially using priority as well. In fact a shutdown message should be safe to clear the queue.

[ghost]

Yes, I realize now the issue is mainly of my own making 😄 still, any plans to detect this and show some sort of language server misbehavior warning to the user? The current semi-quit with panes gone is quite jarring and looks like Atom basically broke itself super bad, while behind the scenes it's actually just the language server breaking itself super bad. Just for your own protection against further mislead user complaints and reports, it might be worth a shot

[damieng]

If it continues to be a source of problems once the typescript language server is fixed then we'll probably have to. Until then I'd rather spend the time and effort on some of the missing features.

[damieng]

It looks like getting traction on this in the upstream language server is slow. We may just have to terminate the child language server process after a couple of seconds.

[ghost]

Yea for me it's the same issue: I use my standard compiler for the LSP backend which isn't really meant for interruption and currently for some queries it's simply dog slow. This isn't good but there is no trivial way for me to solve this, so it might always hang a bit in some situations. Therefore, I'm not sure how practical it is to avoid this in 100% of all cases on the language server side.

Also even for a couple of seconds, it'd be nice if there was some sort of popup stating that Atom is trying to shutdown but still waiting for something. The current window-remains-open-but-panes-gone look is just very confusing and invites the user to try to close it again and do other interfering stuff, so that's probably something that should be avoided

[rmosolgo]

I also discovered this, but in a different context: my server process raised an error, so it never responded to shutdown. I debugged it by checking the logs, but it was a bit tricky to realize that was the error!

[damieng]

I've checked in a fix for this on ide-typescript. I think we're going to let package authors take care of this themselves as they will have to determine a good cut-off time for waiting for their language server to shutdown. In the case of ide-typescript there is a bug where initial processing ignores the incoming shutdown request for a while sourcegraph/javascript-typescript-langserver#367

atom/ide-typescript@d9d2592

[laughedelic]

@damieng after the last update (0.6.7 -> 0.7.0), I noticed that the shutdown behaviour is kind of inconsistent.

Before server-manager would always interrupt the child-process (which would result in 143 exit code), even though after shutdown request-response and exit notification the server is supposed to exit with 0 or 1 (and it does exit in my case always with 0). So I thought that server-manager just doesn't give enough time for the server process to die with dignity.

After the last update, I see that sometimes process ends with exit code 0 and everything is fine, but sometimes it's still interrupted. Is there any way it could be improved?

[damieng]

There should have been no change to shut down behavior between 0.6.7 and 0.7.0 - there is just an extra dispose and some busy signaling. Is it possible your version of Atom also changed?

Atom 1.20 did not allow package deactivation to be async and so Atom would often just quit and leave language servers running for some time after (and some of them didn't ever quit possibly because they got disconnected and into a bad state).

Atom in 1.21 allowed async package deactivation and so everything should shut down totally cleanly. The problem here is that Atom ends up in a half-unloaded state while it waits for the packages to finish. In some cases this can look really bad if it takes a long time (like ide-typescript did so we put timeouts in that specific package).

I can't quite see how you're getting into the state you are though. The sequence is;

1. send shutdown message to server and wait for response
2. send exit message (do not wait)
3. kill the process (do not wait)
4. dispose the connection

I wonder if the problem you're seeing is just one of timing. Ideally step 3 would not be required. The alternative I guess would be something like;

1. send shutdown message and wait 1 second
2. send exit message and wait 1 second
3. kill the process
4. dispose the connection

[laughedelic]

@damieng I'm not sure now about the change of Atom version. It could be that I just didn't notice successful shutdown before or it was a problem on the server side. It seems that the particular problem of this issue got resolved.

The problem i'm talking about is about timing, i.e. step 3 triggering immediately after sending exit notification.

I wonder if the problem you're seeing is just one of timing. Ideally step 3 would not be required. The alternative I guess would be something like ...

Yes, I think that server-manager should give the server a bit of time to shutdown properly. I don't know what is a reasonable delay, but I think that server manager should wait a bit and also check the exit code of the child-process and report an error if it's not 0.

[damieng]

The problem with the error reporting is that typically this is during shutdown so they'll be nowhere to report it.

[laughedelic]

I see. I was thinking about server shutdown on the last tab close when the window is still open (related to #141).