fix(inspector): don't panic if port is not free (#22745)

Closes https://github.com/denoland/deno/issues/22113
Closes https://github.com/denoland/deno/issues/23177
Closes https://github.com/denoland/deno/issues/22883
Closes https://github.com/denoland/deno/issues/22377

cli/args/mod.rs

@@ -1186,14 +1186,20 @@ impl CliOptions {
     }
   }
 
-  pub fn resolve_inspector_server(&self) -> Option<InspectorServer> {
+  pub fn resolve_inspector_server(
+    &self,
+  ) -> Result<Option<InspectorServer>, AnyError> {
     let maybe_inspect_host = self
       .flags
       .inspect
       .or(self.flags.inspect_brk)
       .or(self.flags.inspect_wait);
-    maybe_inspect_host
-      .map(|host| InspectorServer::new(host, version::get_user_agent()))
+
+    let Some(host) = maybe_inspect_host else {
+      return Ok(None);
+    };
+
+    Ok(Some(InspectorServer::new(host, version::get_user_agent())?))
   }
 
   pub fn maybe_lockfile(&self) -> Option<Arc<Mutex<Lockfile>>> {

cli/factory.rs

@@ -672,11 +672,15 @@ impl CliFactory {
     })
   }
 
-  pub fn maybe_inspector_server(&self) -> &Option<Arc<InspectorServer>> {
-    self
-      .services
-      .maybe_inspector_server
-      .get_or_init(|| self.options.resolve_inspector_server().map(Arc::new))
+  pub fn maybe_inspector_server(
+    &self,
+  ) -> Result<&Option<Arc<InspectorServer>>, AnyError> {
+    self.services.maybe_inspector_server.get_or_try_init(|| {
+      match self.options.resolve_inspector_server() {
+        Ok(server) => Ok(server.map(Arc::new)),
+        Err(err) => Err(err),
+      }
+    })
   }
 
   pub async fn module_load_preparer(
@@ -789,7 +793,7 @@ impl CliFactory {
       self.root_cert_store_provider().clone(),
       self.fs().clone(),
       maybe_file_watcher_communicator,
-      self.maybe_inspector_server().clone(),
+      self.maybe_inspector_server()?.clone(),
       self.maybe_lockfile().clone(),
       self.feature_checker().clone(),
       self.create_cli_main_worker_options()?,

cli/lsp/diagnostics.rs

@@ -1692,7 +1692,7 @@ let c: number = "a";
     let cache =
       Arc::new(GlobalHttpCache::new(cache_location, RealDenoCacheEnv));
     let ts_server = TsServer::new(Default::default(), cache);
-    ts_server.start(None);
+    ts_server.start(None).unwrap();
 
     // test enabled
     {

cli/lsp/language_server.rs

@@ -971,9 +971,14 @@ impl Inner {
       self.config.update_capabilities(&params.capabilities);
     }
 
-    self
+    if let Err(e) = self
       .ts_server
-      .start(self.config.internal_inspect().to_address());
+      .start(self.config.internal_inspect().to_address())
+    {
+      lsp_warn!("{}", e);
+      self.client.show_message(MessageType::ERROR, e);
+      return Err(tower_lsp::jsonrpc::Error::internal_error());
+    };
 
     self.update_debug_flag();
     self.refresh_workspace_files();

cli/lsp/tsc.rs

@@ -36,6 +36,7 @@ use crate::util::path::to_percent_decoded_str;
 use dashmap::DashMap;
 use deno_ast::MediaType;
 use deno_core::anyhow::anyhow;
+use deno_core::anyhow::Context as _;
 use deno_core::error::custom_error;
 use deno_core::error::AnyError;
 use deno_core::futures::FutureExt;
@@ -268,17 +269,20 @@ impl TsServer {
     }
   }
 
-  pub fn start(&self, inspector_server_addr: Option<String>) {
-    let maybe_inspector_server = inspector_server_addr.and_then(|addr| {
-      let addr: SocketAddr = match addr.parse() {
-        Ok(addr) => addr,
-        Err(err) => {
-          lsp_warn!("Invalid inspector server address \"{}\": {}", &addr, err);
-          return None;
-        }
-      };
-      Some(Arc::new(InspectorServer::new(addr, "deno-lsp-tsc")))
-    });
+  pub fn start(
+    &self,
+    inspector_server_addr: Option<String>,
+  ) -> Result<(), AnyError> {
+    let maybe_inspector_server = match inspector_server_addr {
+      Some(addr) => {
+        let addr: SocketAddr = addr.parse().with_context(|| {
+          format!("Invalid inspector server address \"{}\"", &addr)
+        })?;
+        let server = InspectorServer::new(addr, "deno-lsp-tsc")?;
+        Some(Arc::new(server))
+      }
+      None => None,
+    };
     *self.inspector_server.lock() = maybe_inspector_server.clone();
     // TODO(bartlomieju): why is the join_handle ignored here? Should we store it
     // on the `TsServer` struct.
@@ -295,6 +299,7 @@ impl TsServer {
         maybe_inspector_server,
       )
     });
+    Ok(())
   }
 
   pub async fn project_changed(
@@ -4750,7 +4755,7 @@ mod tests {
       Arc::new(mock_state_snapshot(sources, &location, config).await);
     let performance = Arc::new(Performance::default());
     let ts_server = TsServer::new(performance, cache.clone());
-    ts_server.start(None);
+    ts_server.start(None).unwrap();
     (ts_server, snapshot, cache)
   }
 

runtime/inspector_server.rs

@@ -2,6 +2,8 @@
 
 // Alias for the future `!` type.
 use core::convert::Infallible as Never;
+use deno_core::anyhow::Context;
+use deno_core::error::AnyError;
 use deno_core::futures::channel::mpsc;
 use deno_core::futures::channel::mpsc::UnboundedReceiver;
 use deno_core::futures::channel::mpsc::UnboundedSender;
@@ -45,27 +47,38 @@ pub struct InspectorServer {
 }
 
 impl InspectorServer {
-  pub fn new(host: SocketAddr, name: &'static str) -> Self {
+  pub fn new(host: SocketAddr, name: &'static str) -> Result<Self, AnyError> {
     let (register_inspector_tx, register_inspector_rx) =
       mpsc::unbounded::<InspectorInfo>();
 
     let (shutdown_server_tx, shutdown_server_rx) = broadcast::channel(1);
 
+    let tcp_listener =
+      std::net::TcpListener::bind(host).with_context(|| {
+        format!("Failed to start inspector server at \"{}\"", host)
+      })?;
+    tcp_listener.set_nonblocking(true)?;
+
     let thread_handle = thread::spawn(move || {
       let rt = crate::tokio_util::create_basic_runtime();
       let local = tokio::task::LocalSet::new();
       local.block_on(
         &rt,
-        server(host, register_inspector_rx, shutdown_server_rx, name),
+        server(
+          tcp_listener,
+          register_inspector_rx,
+          shutdown_server_rx,
+          name,
+        ),
       )
     });
 
-    Self {
+    Ok(Self {
       host,
       register_inspector_tx,
       shutdown_server_tx: Some(shutdown_server_tx),
       thread_handle: Some(thread_handle),
-    }
+    })
   }
 
   pub fn register_inspector(
@@ -220,7 +233,7 @@ fn handle_json_version_request(
 }
 
 async fn server(
-  host: SocketAddr,
+  listener: std::net::TcpListener,
   register_inspector_rx: UnboundedReceiver<InspectorInfo>,
   shutdown_server_rx: broadcast::Receiver<()>,
   name: &str,
@@ -261,7 +274,7 @@ async fn server(
   });
 
   // Create the server manually so it can use the Local Executor
-  let listener = match TcpListener::bind(&host).await {
+  let listener = match TcpListener::from_std(listener) {
     Ok(l) => l,
     Err(err) => {
       eprintln!("Cannot start inspector server: {:?}", err);

tests/integration/inspector_tests.rs

@@ -465,7 +465,7 @@ async fn inspector_port_collision() {
     .lines()
     .map(|r| r.unwrap())
     .inspect(|line| assert!(!line.contains("Debugger listening")))
-    .find(|line| line.contains("Cannot start inspector server"));
+    .find(|line| line.contains("Failed to start inspector server"));
   assert!(stderr_2_error_message.is_some());
 
   child1.kill().unwrap();