mux: don't kill panes when the domain is detached!

refs: #1993

docs/changelog.md

@@ -53,6 +53,7 @@ As features stabilize some brief notes about them will accumulate here.
 * Unable to use `ALT` as a modifier for the `leader` key. [#1958](https://github.com/wez/wezterm/issues/1958)
 * IME Candidate window position was incorrect. Thanks to [@kumattau](https://github.com/kumattau)! [#1976](https://github.com/wez/wezterm/pull/1976)
 * Prevent panic for some classes of invalid input, found while fuzzing. Thanks to [@5225225](https://github.com/5225225)! [#1990](https://github.com/wez/wezterm/pull/1990) [#1986](https://github.com/wez/wezterm/pull/1986)
+* Detaching an ssh multiplexer domain sometimes killed the associated panes! [#1993](https://github.com/wez/wezterm/issues/1993)
 
 ### 20220408-101518-b908e2dd
 

mux/src/lib.rs

@@ -867,7 +867,7 @@ impl Mux {
             }
         }
 
-        log::error!("domain detached panes: {:?}", dead_panes);
+        log::info!("domain detached panes: {:?}", dead_panes);
         for pane_id in dead_panes {
             self.remove_pane_internal(pane_id);
         }

wezterm-client/src/client.rs

@@ -573,7 +573,7 @@ impl Reconnectable {
             format!("{} cli --prefer-mux --no-auto-start proxy", proxy_bin)
         };
         ui.output_str(&format!("Running: {}\n", cmd));
-        log::error!("going to run {}", cmd);
+        log::info!("going to run {}", cmd);
 
         let exec = smol::block_on(sess.exec(&cmd, None))?;
 

wezterm-client/src/domain.rs

@@ -261,7 +261,7 @@ impl ClientDomain {
     }
 
     pub fn perform_detach(&self) {
-        log::error!("detached domain {}", self.local_domain_id);
+        log::info!("detached domain {}", self.local_domain_id);
         self.inner.borrow_mut().take();
         let mux = Mux::get().unwrap();
         mux.domain_was_detached(self.local_domain_id);

wezterm-client/src/pane/clientpane.rs

@@ -373,34 +373,58 @@ impl Pane for ClientPane {
         let client = Arc::clone(&self.client);
         let remote_pane_id = self.remote_pane_id;
         let local_domain_id = self.client.local_domain_id;
-        promise::spawn::spawn(async move {
-            client
-                .client
-                .kill_pane(KillPane {
-                    pane_id: remote_pane_id,
-                })
-                .await?;
 
-            // Arrange to resync the layout, to avoid artifacts
-            // <https://github.com/wez/wezterm/issues/1277>.
-            // We need a short delay to avoid racing with the observable
-            // effects of killing the pane.
-            // <https://github.com/wez/wezterm/issues/1752#issuecomment-1088269363>
-            smol::Timer::after(std::time::Duration::from_millis(200)).await;
+        // We only want to ask the server to kill the pane if the user
+        // explicitly requested it to die.
+        // Domain detaching can implicitly call Pane::kill on the panes
+        // in the domain, so we need to check here whether the domain is
+        // in the detached state; if so then we must skip sending the
+        // kill to the server.
+        let mut send_kill = true;
+
+        {
             let mux = Mux::get().expect("called on main thread");
-            let client_domain = mux
-                .get_domain(local_domain_id)
-                .ok_or_else(|| anyhow::anyhow!("no such domain {}", local_domain_id))?;
-            let client_domain = client_domain
-                .downcast_ref::<ClientDomain>()
-                .ok_or_else(|| {
-                    anyhow::anyhow!("domain {} is not a ClientDomain instance", local_domain_id)
-                })?;
+            if let Some(client_domain) = mux.get_domain(local_domain_id) {
+                if client_domain.state() == mux::domain::DomainState::Detached {
+                    send_kill = false;
+                }
+            }
+        }
 
-            client_domain.resync().await?;
-            anyhow::Result::<()>::Ok(())
-        })
-        .detach();
+        if send_kill {
+            promise::spawn::spawn(async move {
+                client
+                    .client
+                    .kill_pane(KillPane {
+                        pane_id: remote_pane_id,
+                    })
+                    .await?;
+
+                // Arrange to resync the layout, to avoid artifacts
+                // <https://github.com/wez/wezterm/issues/1277>.
+                // We need a short delay to avoid racing with the observable
+                // effects of killing the pane.
+                // <https://github.com/wez/wezterm/issues/1752#issuecomment-1088269363>
+                smol::Timer::after(std::time::Duration::from_millis(200)).await;
+                let mux = Mux::get().expect("called on main thread");
+                let client_domain = mux
+                    .get_domain(local_domain_id)
+                    .ok_or_else(|| anyhow::anyhow!("no such domain {}", local_domain_id))?;
+                let client_domain =
+                    client_domain
+                        .downcast_ref::<ClientDomain>()
+                        .ok_or_else(|| {
+                            anyhow::anyhow!(
+                                "domain {} is not a ClientDomain instance",
+                                local_domain_id
+                            )
+                        })?;
+
+                client_domain.resync().await?;
+                anyhow::Result::<()>::Ok(())
+            })
+            .detach();
+        }
         // Explicitly mark ourselves as dead.
         // Ideally we'd discover this later when polling the
         // status, but killing the pane prevents the server