Repository: denoland/deno (language: rust)

Issue:
LSP not responding when editor quits and sends shutdown request
When quitting my EDITOR ([BBEdit](https://www.barebones.com/products/bbedit/)), it hangs waiting for `deno lsp`. 

I have reported the issue to BareBones. They advised that it's possibly due to LSP server not responding correctly to the application's request to shut itself down. 

- https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#shutdown

Does `deno lsp` respond to the `shutdown` request?

Is there a config setting for handling the `shutdown` request?


Discussion / hints:
The shutdown handler we have just tries to acquire the global mutex it has (ensuring in-flight operations finish) and then responds with `Ok(())`. If it's really hanging, maybe it's because a previous request going either way is on-going. I don't think this is been a problem with other editors though. Unfortunately I can't investigate very far since BBEdit's source isn't public.

@cngarrison Does this happen with a bare minimum reproduction? E.g. open an empty ts file in a fresh directory, plus whichever method you're using to enable the Deno LSP, and try to exit. 
@nayeemrmn Thanks for that feedback.

I launched BBEdit with `deno` LSP server enabled and no documents open. I then quit BBEdit and it quit cleanly.

I launched BBEdit, opened a new project directory (file browser), created an (empty) index.ts file, then quit BBEdit. The hang returned. 

The team at BareBones is very good, and I have flagged this issue with them. Please advise if there is any further testing I can help with.
> When quitting my EDITOR ([BBEdit](https://www.barebones.com/products/bbedit/)), it hangs waiting for `deno lsp`.
> 
> I have reported the issue to BareBones. They advised that it's possibly due to LSP server not responding correctly to the application's request to shut itself down.

@sigmaSd Do you know of any quirks that might relate to this?
For background (and in case it helps focus the investigation):

When stopping a language server (at application quit or for certain configuration changes), BBEdit sends a `shutdown` request to the server and waits for a response.

After receiving a response to `shutdown`, BBEdit then sends the server an `exit` notification.

Then, BBEdit waits for the server process to exit. This is done because some language servers actually do nothing at `shutdown` time, and instead do all of their cleanup (index database commits, and the like) when they receive an `exit`, and in some cases this can take a number of seconds. (An example of this is found in [clangd](https://github.com/clangd/clangd/issues/855) .)

In the case of `deno`, it responds normally (and rapidly) to `shutdown`, but after receiving the `exit` notification, the `deno` process never terminates.

If `deno` has any work to do when shutting the server down (such as committing an index database, or other housekeeping), the `shutdown` handler is the time to do this; and the `exit` notification handler simply needs to terminate the server process.

Hope this helps. :-)
Its never been a problem on nvim/helix, so I don't have other informations regarding this.
@siegel Is there a chance BBEdit isn't closing the LSP process's stdin?
> @siegel Is there a chance BBEdit isn't closing the LSP process's stdin?

Probably. :-) BBEdit does not explicitly close the pipes until after the server process has exited as requested.

Is there something in the LSP specification that states that the client is expected to close `stdin` after sending the `exit` notification?
> Is there something in the LSP specification that states that the client is expected to close `stdin` after sending the `exit` notification?

Nope. Based on the wording at https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#exit we should exit without needing that. But given that this issue occurs specifically between Deno and BBEdit, maybe both actions should be taken

@ebkalderon Could `Server::serve()` be changed to break its loop on exit notifications without waiting for stdin to close, so our program can finish up as normal? Or is there something we can do currently?
@nayeemrmn Certainly, this could (and should) definitely be changed. In theory, `Server::serve()` should already gracefully shut down when it receives an `exit` message, regardless of the state of `stdin`, but if it doesn't, then there must be a bug in the logic somewhere. Please feel free to open a bug report on the `tower-lsp` repo to investigate further. Thanks for bringing this to my attention, folks!
> Please feel free to open a bug report on the `tower-lsp` repo to investigate further. Thanks for bringing this to my attention, folks!

Done! [Your #399](https://github.com/ebkalderon/tower-lsp/issues/399) is it. And I appreciate you taking an interest in looking into it. :-)
I'm experiencing the same issue in Nova with the built-in LanguageClient. Manually restarting the LSP client often causes the server to hang, and after a time a stream of `The request was cancelled.` messages come through.


Fix patch:
diff --git a/Cargo.lock b/Cargo.lock
index dd7c3bd25173bc..1f831d22a13e36 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1570,9 +1570,9 @@ dependencies = [
 
 [[package]]
 name = "deno-tower-lsp-macros"
-version = "0.10.0"
+version = "0.11.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "e017686632f7908cde3e02cd1598e25488f57448600a09211834cfd547b9ec39"
+checksum = "61083feefc9c29a3f45670be6b60001ec209286c861b6830cd30ba4af9a6e9d8"
 dependencies = [
  "proc-macro2",
  "quote",
@@ -2691,9 +2691,9 @@ dependencies = [
 
 [[package]]
 name = "deno_tower_lsp"
-version = "0.3.0"
+version = "0.4.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "c58d75640e2edffc77929064f2f13c308e549b4b5c42ea6159b0607add856675"
+checksum = "c29dce82d67177bbc36468f64cddd8c25242fc31b99369ef0126fa7fad64c5ec"
 dependencies = [
  "async-trait",
  "auto_impl",
diff --git a/Cargo.toml b/Cargo.toml
index 81980a369f0101..43820cdbf171ae 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -218,7 +218,7 @@ tokio-socks = "0.5.1"
 tokio-util = "0.7.4"
 tower = { version = "0.5.2", default-features = false, features = ["retry", "util"] }
 tower-http = { version = "0.6.1", features = ["decompression-br", "decompression-gzip"] }
-tower-lsp = { package = "deno_tower_lsp", version = "=0.3.0", features = ["proposed"] }
+tower-lsp = { package = "deno_tower_lsp", version = "=0.4.2", features = ["proposed"] }
 tower-service = "0.3.2"
 twox-hash = "=1.6.3"
 url = { version = "2.5", features = ["serde", "expose_internals"] }
diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs
index df7c00d3bdb218..9d253c250ff7c3 100644
--- a/cli/lsp/language_server.rs
+++ b/cli/lsp/language_server.rs
@@ -137,7 +137,6 @@ pub struct LanguageServer {
   /// https://github.com/Microsoft/language-server-protocol/issues/567#issuecomment-2085131917
   init_flag: AsyncFlag,
   performance: Arc<Performance>,
-  shutdown_flag: AsyncFlag,
 }
 
 /// Snapshot of the state used by TSC.
@@ -227,7 +226,7 @@ pub struct Inner {
 }
 
 impl LanguageServer {
-  pub fn new(client: Client, shutdown_flag: AsyncFlag) -> Self {
+  pub fn new(client: Client) -> Self {
     let performance = Arc::new(Performance::default());
     Self {
       client: client.clone(),
@@ -237,7 +236,6 @@ impl LanguageServer {
       ))),
       init_flag: Default::default(),
       performance,
-      shutdown_flag,
     }
   }
 
@@ -3566,7 +3564,6 @@ impl tower_lsp::LanguageServer for LanguageServer {
   }
 
   async fn shutdown(&self) -> LspResult<()> {
-    self.shutdown_flag.raise();
     Ok(())
   }
 
diff --git a/cli/lsp/mod.rs b/cli/lsp/mod.rs
index 5df7da61d449e4..1f09c614f184af 100644
--- a/cli/lsp/mod.rs
+++ b/cli/lsp/mod.rs
@@ -1,7 +1,6 @@
 // Copyright 2018-2025 the Deno authors. MIT license.
 
 use deno_core::error::AnyError;
-use deno_core::unsync::spawn;
 pub use repl::ReplCompletionItem;
 pub use repl::ReplLanguageServer;
 use tower_lsp::LspService;
@@ -9,7 +8,6 @@ use tower_lsp::Server;
 
 use self::diagnostics::should_send_diagnostic_batch_index_notifications;
 use crate::lsp::language_server::LanguageServer;
-use crate::util::sync::AsyncFlag;
 
 mod analysis;
 mod cache;
@@ -44,12 +42,8 @@ pub async fn start() -> Result<(), AnyError> {
   let stdin = tokio::io::stdin();
   let stdout = tokio::io::stdout();
 
-  let shutdown_flag = AsyncFlag::default();
   let builder = LspService::build(|client| {
-    language_server::LanguageServer::new(
-      client::Client::from_tower(client),
-      shutdown_flag.clone(),
-    )
+    language_server::LanguageServer::new(client::Client::from_tower(client))
   })
   .custom_method(
     lsp_custom::PERFORMANCE_REQUEST,
@@ -76,19 +70,9 @@ pub async fn start() -> Result<(), AnyError> {
   };
 
   let (service, socket, pending) = builder.finish();
-
-  // TODO(nayeemrmn): This shutdown flag is a workaround for
-  // https://github.com/denoland/deno/issues/20700. Remove when
-  // https://github.com/ebkalderon/tower-lsp/issues/399 is fixed.
-  // Force end the server 8 seconds after receiving a shutdown request.
-  tokio::select! {
-    biased;
-    _ = Server::new(stdin, stdout, socket, pending).concurrency_level(32).serve(service) => {}
-    _ = spawn(async move {
-      shutdown_flag.wait_raised().await;
-      tokio::time::sleep(std::time::Duration::from_secs(8)).await;
-    }) => {}
-  }
-
+  Server::new(stdin, stdout, socket, pending)
+    .concurrency_level(32)
+    .serve(service)
+    .await;
   Ok(())
 }
diff --git a/cli/lsp/repl.rs b/cli/lsp/repl.rs
index d247822837cdfd..7aec4c06766164 100644
--- a/cli/lsp/repl.rs
+++ b/cli/lsp/repl.rs
@@ -65,10 +65,8 @@ impl ReplLanguageServer {
     super::logging::set_lsp_log_level(log::Level::Debug);
     super::logging::set_lsp_warn_level(log::Level::Debug);
 
-    let language_server = super::language_server::LanguageServer::new(
-      Client::new_for_repl(),
-      Default::default(),
-    );
+    let language_server =
+      super::language_server::LanguageServer::new(Client::new_for_repl());
 
     let cwd_uri = get_cwd_uri()?;
 


Test patch:
diff --git a/tests/integration/lsp_tests.rs b/tests/integration/lsp_tests.rs
index 6cb22406e59eb0..059bb4f1209229 100644
--- a/tests/integration/lsp_tests.rs
+++ b/tests/integration/lsp_tests.rs
@@ -27,6 +27,7 @@ fn lsp_startup_shutdown() {
   let mut client = context.new_lsp_command().build();
   client.initialize_default();
   client.shutdown();
+  assert!(client.wait_exit().unwrap().success());
 }
 
 #[test]
diff --git a/tests/util/server/src/lsp.rs b/tests/util/server/src/lsp.rs
index 1d7a7f8cc3ec7f..86594174724b85 100644
--- a/tests/util/server/src/lsp.rs
+++ b/tests/util/server/src/lsp.rs
@@ -731,7 +731,7 @@ impl Drop for LspClient {
         self.child.kill().unwrap();
         let _ = self.child.wait();
       }
-      Ok(Some(status)) => panic!("deno lsp exited unexpectedly {status}"),
+      Ok(Some(_)) => {}
       Err(e) => panic!("pebble error: {e}"),
     }
   }
@@ -1002,6 +1002,10 @@ impl LspClient {
     self.write_notification("exit", json!(null));
   }
 
+  pub fn wait_exit(&mut self) -> std::io::Result<std::process::ExitStatus> {
+    self.child.wait()
+  }
+
   // it's flaky to assert for a notification because a notification
   // might arrive a little later, so only provide a method for asserting
   // that there is no notification