mux: fix update issue

The heart of the issue here was due to the window-reuse logic that
tries to reuse a GUI window that is no longer associated with a mux
window.

Each GUI window subscribes to the mux for mux events, but it filters
according to is understanding of the mux_window_id that it is associated
with.

The GUI frontend maintains an mapping of GUI and mux window so that it
knows when to reuse a GUI window and when to close it.

When connecting to a remote mux, wezterm spawns a temporary connection
progress window.  Once connected, workspace reconiliation is triggered
and decides that this window can be used for something else.

As part of workspace reconciliation, this mapping can be adjusted and
the frontend will notify a GUI window that its mux window has changed.

However, that updated mux window was not visible to the mux notification
subscription so the effect was that a variety of notifications were
effectively ignored, including updates from a remote mux when the output
was changed.

To make matters worse, the workspace reconciliation could "double-tap"
window creation and create excess windows only to later realize they
weren't needed and close them out again.

This commit addresses both of these concerns.

refs: #1841
refs: #1814

wezterm-client/src/client.rs

@@ -1157,7 +1157,7 @@ impl Client {
     rpc!(resize, Resize, UnitResponse);
     rpc!(set_zoomed, SetPaneZoomed, UnitResponse);
     rpc!(
-        get_tab_render_changes,
+        get_pane_render_changes,
         GetPaneRenderChanges,
         LivenessResponse
     );

wezterm-client/src/pane/renderable.rs

@@ -303,6 +303,11 @@ impl RenderableInner {
         delta: GetPaneRenderChangesResponse,
         bonus_lines: Vec<(StableRowIndex, Line)>,
     ) {
+        log::trace!(
+            "apply_changes_to_surface local={} remote={}",
+            self.local_pane_id,
+            self.remote_pane_id
+        );
         let now = Instant::now();
         self.poll_interval = BASE_POLL_INTERVAL;
         self.last_recv_time = now;
@@ -343,7 +348,12 @@ impl RenderableInner {
         self.dimensions = delta.dimensions;
         self.title = delta.title;
         self.working_dir = delta.working_dir.map(Into::into);
-        log::trace!("server says: seqno from {} -> {}", self.seqno, delta.seqno);
+        log::trace!(
+            "server says: seqno from {} -> {} for local_pane_id={}",
+            self.seqno,
+            delta.seqno,
+            self.local_pane_id
+        );
         self.seqno = delta.seqno;
 
         let config = configuration();
@@ -353,6 +363,10 @@ impl RenderableInner {
             dirty.remove(stable_row);
         }
 
+        log::trace!(
+            "apply_changes_to_surface: Generate PaneOutput event for local={}",
+            self.local_pane_id
+        );
         Mux::get()
             .unwrap()
             .notify(mux::MuxNotification::PaneOutput(self.local_pane_id));
@@ -393,7 +407,10 @@ impl RenderableInner {
             if self.fetch_limiter.non_blocking_admittance_check(1) {
                 self.schedule_fetch_lines(to_fetch, now);
             } else {
-                log::trace!("exceeded throttle, drop {:?}", to_fetch);
+                log::warn!(
+                    "exceeded fetch throttle, drop {:?} and mark stale",
+                    to_fetch
+                );
                 for r in to_fetch.iter() {
                     for stable_row in r.clone() {
                         self.make_stale(stable_row);
@@ -556,6 +573,10 @@ impl RenderableInner {
                 }
             }
         }
+        log::trace!(
+            "Generate PaneOutput event for local_pane_id={}",
+            local_pane_id
+        );
         mux.notify(mux::MuxNotification::PaneOutput(local_pane_id));
         Ok(())
     }
@@ -582,7 +603,7 @@ impl RenderableInner {
         promise::spawn::spawn(async move {
             let alive = match client
                 .client
-                .get_tab_render_changes(GetPaneRenderChanges {
+                .get_pane_render_changes(GetPaneRenderChanges {
                     pane_id: remote_pane_id,
                 })
                 .await

wezterm-gui/src/frontend.rs

@@ -7,7 +7,7 @@ use mux::client::ClientId;
 use mux::window::WindowId as MuxWindowId;
 use mux::{Mux, MuxNotification};
 use std::cell::RefCell;
-use std::collections::BTreeMap;
+use std::collections::{BTreeMap, HashSet};
 use std::rc::Rc;
 use std::sync::Arc;
 use wezterm_term::{Alert, ClipboardSelection};
@@ -16,6 +16,7 @@ use wezterm_toast_notification::*;
 pub struct GuiFrontEnd {
     connection: Rc<Connection>,
     switching_workspaces: RefCell<bool>,
+    spawned_mux_window: RefCell<HashSet<MuxWindowId>>,
     known_windows: RefCell<BTreeMap<Window, MuxWindowId>>,
     client_id: Arc<ClientId>,
 }
@@ -36,6 +37,7 @@ impl GuiFrontEnd {
         let front_end = Rc::new(GuiFrontEnd {
             connection,
             switching_workspaces: RefCell::new(false),
+            spawned_mux_window: RefCell::new(HashSet::new()),
             known_windows: RefCell::new(BTreeMap::new()),
             client_id: client_id.clone(),
         });
@@ -188,7 +190,7 @@ impl GuiFrontEnd {
 
         let mut mux_windows = mux_windows.into_iter();
 
-        for (window, _old_id) in unused.into_iter() {
+        for (window, old_id) in unused.into_iter() {
             if let Some(mux_window_id) = mux_windows.next() {
                 window.notify(TermWindowNotif::SwitchToMuxWindow(mux_window_id));
                 windows.insert(window, mux_window_id);
@@ -196,18 +198,37 @@ impl GuiFrontEnd {
                 // We have more windows than are in the new workspace;
                 // we no longer need this one!
                 window.close();
+                front_end().spawned_mux_window.borrow_mut().remove(&old_id);
             }
         }
 
+        log::trace!("reconcile: windows -> {:?}", windows);
         *self.known_windows.borrow_mut() = windows;
 
         // then spawn any new windows that are needed
         promise::spawn::spawn(async move {
             while let Some(mux_window_id) = mux_windows.next() {
+                if front_end().has_mux_window(mux_window_id)
+                    || front_end()
+                        .spawned_mux_window
+                        .borrow()
+                        .contains(&mux_window_id)
+                {
+                    continue;
+                }
+                front_end()
+                    .spawned_mux_window
+                    .borrow_mut()
+                    .insert(mux_window_id);
+                log::trace!("Creating TermWindow for mux_window_id={}", mux_window_id);
                 if let Err(err) = TermWindow::new_window(mux_window_id).await {
                     log::error!("Failed to create window: {:#}", err);
                     let mux = Mux::get().expect("switching_workspaces to trigger on main thread");
                     mux.kill_window(mux_window_id);
+                    front_end()
+                        .spawned_mux_window
+                        .borrow_mut()
+                        .remove(&mux_window_id);
                 }
             }
             *front_end().switching_workspaces.borrow_mut() = false;
@@ -215,6 +236,15 @@ impl GuiFrontEnd {
         .detach();
     }
 
+    fn has_mux_window(&self, mux_window_id: MuxWindowId) -> bool {
+        for &mux_id in self.known_windows.borrow().values() {
+            if mux_id == mux_window_id {
+                return true;
+            }
+        }
+        false
+    }
+
     pub fn switch_workspace(&self, workspace: &str) {
         let mux = Mux::get().expect("mux started and running on main thread");
         mux.set_active_workspace_for_client(&self.client_id, workspace);

wezterm-gui/src/termwindow/mod.rs

@@ -307,6 +307,7 @@ pub struct TermWindow {
     /// Terminal dimensions
     terminal_size: PtySize,
     pub mux_window_id: MuxWindowId,
+    pub mux_window_id_for_subscriptions: Arc<Mutex<MuxWindowId>>,
     pub render_metrics: RenderMetrics,
     render_state: Option<RenderState>,
     input_map: InputMap,
@@ -730,6 +731,7 @@ impl TermWindow {
             palette: None,
             focused: None,
             mux_window_id,
+            mux_window_id_for_subscriptions: Arc::new(Mutex::new(mux_window_id)),
             fonts: Rc::clone(&fontconfig),
             render_metrics,
             dimensions,
@@ -1091,6 +1093,7 @@ impl TermWindow {
             }
             TermWindowNotif::SwitchToMuxWindow(mux_window_id) => {
                 self.mux_window_id = mux_window_id;
+                *self.mux_window_id_for_subscriptions.lock().unwrap() = mux_window_id;
                 self.pane_state.borrow_mut().clear();
                 self.tab_state.borrow_mut().clear();
                 self.current_highlight.take();
@@ -1178,6 +1181,11 @@ impl TermWindow {
                 let mux = Mux::get().expect("mux is calling us");
                 if mux.get_window(mux_window_id).is_none() {
                     // Something inconsistent: cancel subscription
+                    log::error!(
+                        "PaneOutput: wanted mux_window_id={} from mux, but \
+                         was not found, cancel mux subscription",
+                        mux_window_id
+                    );
                     return false;
                 }
                 let _ = pane_id;
@@ -1217,10 +1225,11 @@ impl TermWindow {
 
     fn subscribe_to_pane_updates(&self) {
         let window = self.window.clone().expect("window to be valid on startup");
-        let mux_window_id = self.mux_window_id;
+        let mux_window_id = Arc::clone(&self.mux_window_id_for_subscriptions);
         let mux = Mux::get().expect("mux started and running on main thread");
         let dead = Arc::new(AtomicBool::new(false));
         mux.subscribe(move |n| {
+            let mux_window_id = *mux_window_id.lock().unwrap();
             Self::mux_pane_output_event_callback(n, &window, mux_window_id, &dead)
         });
     }