hashicorp / terraform-ls

Terraform Language Server
Mozilla Public License 2.0
997 stars 132 forks source link

Server process fails to exit in response to `exit` notification from client #791

Open siegel opened 2 years ago

siegel commented 2 years ago

Server Version

0.25.2

Terraform Version

zsh: command not found: terraform

Client Version

BBEdit 14.1 (416142)

Terraform Configuration Files

n/a

Log Output

Terraform debug log is here.

BBEdit's client-side log is here.

Expected Behavior

When quitting BBEdit, the application sends an exit protocol notification to the server. According to the specification, the server is expected to exit. In order to ensure clean shutdown, the client (BBEdit) waits for the server process to exit.

Actual Behavior

The terraform-ls process does not exit, consequently BBEdit waits forever (unless the user manually kills the terraform-ls process).

Steps to Reproduce

  1. Install the Terraform language module from its GitHub repo.
  2. Configure BBEdit to use terraform-ls as described here.
  3. Start (or restart) the application and open any .tf file. => Observe that terraform-ls starts and appears in the process list
  4. Quit BBEdit. => At this point BBEdit will stall, waiting for terraform-ls to exit, which never happens.
radeksimko commented 2 years ago

I have not tried to reproduce this specifically with BBEdit yet, but the logs contain what I'd expect. The last line clearly confirms that the exit notification was received and typically, the LS does not log anything else after that, exactly because it would exit.

Just for completeness, the patterns of your file paths would suggest you are on macOS - am I right?

The terraform-ls process does not exit, consequently BBEdit waits forever (unless the user manually kills the terraform-ls process).

Can you confirm it is the exact same PID still hanging around? There is a log line such as langserver.go:94: Starting server (pid XYZ; concurrency: 5) ... so can you check for that exact PID e.g. in ps?

ps -f -p XYZ

I remember dealing with somewhat related problem in Sublime Text, which would send SIGKILL to LS https://github.com/sublimelsp/LSP/issues/1660

So it is theoretically possible that other editors just mask a bug by killing the process at some point.

siegel commented 2 years ago

@radeksimko I appreciate you looking into this.

Just for completeness, the patterns of your file paths would suggest you are on macOS - am I right?

Yes, that is correct.

Can you confirm it is the exact same PID still hanging around?

Confirmed. From the log at startup:

2022/02/17 12:41:58 langserver.go:94: Starting server (pid 63221; concurrency: 5) ...

And while waiting for exit:

(base) siegel@Stormwind /tmp % ps -f -p 63221
  UID   PID  PPID   C STIME   TTY           TIME CMD
  501 63221 62489   0 12:41PM ??         0:00.12 /opt/homebrew/bin/terraform-ls serve -log-file=/tmp/terraform-ls-{{pid}}.log

So it is theoretically possible that other editors just mask a bug by killing the process at some point.

This would not surprise me; I've seen a couple of servers fail to exit (at all) when receiving an exit notification from the client, and I suspect that most clients deal with this by killing the server outright if it doesn't exit fast enough to suit them.

That's a bad idea for some servers, like clangd. It does its cleanup at exit instead of shutdown, so many seconds can elapse between receipt of the exit notification and the actual process exit. (See https://github.com/clangd/clangd/issues/855 and the discussion there.)

siegel commented 2 years ago

To follow up on this: If I kill 63221 in Terminal while waiting for the process, then BBEdit finishes quitting normally (as expected), and the following appears in the detailed log file (the first line being left in for context):

2022/02/17 12:43:42 rpc_logger.go:29: Incoming notification for "exit": 
2022/02/17 12:45:33 signal_cancel.go:20: Cancellation signal (terminated) received
2022/02/17 12:45:33 langserver.go:107: Stopping server (pid 63221) ...
2022/02/17 12:45:33 opts.go:254: Server signaled to stop with err=the server has been stopped
2022/02/17 12:45:33 langserver.go:111: Server (pid 63221) stopped.
2022/02/17 12:45:33 opts.go:254: Error reading from client: the server has been stopped

The time interval between the first two lines is the amount of time I waited for the server to exit before using kill 63221 (which sends SIGTERM).

So it certainly seems as though something's getting the server stuck when it receives the exit notification from the client, although whatever mechanics are in place for handling signals seem to correctly be unwinding and terminating the server.

thewellington commented 2 years ago

Has there been any progress on this issue. I am still experiencing this using terraform-ls 0.29.2 and BBEdit 14 on both Apple Silicon and Intel Macs

siegel commented 2 years ago

There is an expert preference (BBEdit 14.5 and later) that you can use to work around this:

defaults write com.barebones.bbedit ForceQuitLSPServerAfterExit_terraform-ls -int N

The N should be a nonzero positive integer; it is the maximum number of seconds that BBEdit will wait for the process to exit before giving up and killing it outright. 1 is probably sufficient.

thewellington commented 2 years ago

Works swimmingly on both architectures. Thanks Rich! Unfortunate that the solution can't come from the LSP itself.