diff --git a/crates/tty7-core/src/core/shells.rs b/crates/tty7-core/src/core/shells.rs index 8fa55afb..32ffba12 100644 --- a/crates/tty7-core/src/core/shells.rs +++ b/crates/tty7-core/src/core/shells.rs @@ -463,19 +463,103 @@ mod wsl_tests { "registry default {default:?} not in {installed:?}" ); } + + /// Windows Terminal drops `Modern = 1` distros from this very key, because + /// they hand it a profile fragment separately and it would otherwise list + /// them twice. Copying that filter here would hide the ordinary distro on + /// an up-to-date machine — on the box this was written on, the only one. + #[test] + fn a_modern_distro_is_still_offered() { + let installed = super::wsl_distros(); + if installed.is_empty() { + eprintln!("skipping: no WSL distributions installed"); + return; + } + let modern: Vec = super::registry_user_subkeys(super::LXSS) + .unwrap_or_default() + .iter() + .filter(|guid| { + super::registry_user_dword(&format!(r"{}\{guid}", super::LXSS), "Modern") == Some(1) + }) + .filter_map(|guid| { + super::registry_user_string(&format!(r"{}\{guid}", super::LXSS), "DistributionName") + }) + .filter(|name| super::worth_offering(name)) + .collect(); + if modern.is_empty() { + eprintln!("skipping: no modern WSL distributions installed"); + return; + } + for name in &modern { + assert!( + installed.contains(name), + "modern distro {name:?} was dropped from {installed:?}" + ); + } + } + + #[test] + fn listing_the_distros_does_not_wait_on_the_wsl_service() { + // Only the registry answer is meant to be fast. When there is none the + // fallback to `wsl.exe` is doing exactly what it exists for, and timing + // it would fail this test on every machine without WSL installed. + if super::registered_wsl_distros().is_none() { + eprintln!("skipping: the registry has no distro list to read"); + return; + } + let started = std::time::Instant::now(); + let _ = super::wsl_distros(); + let elapsed = started.elapsed(); + assert!( + elapsed < std::time::Duration::from_millis(500), + "the listing went to `wsl.exe` after all: {elapsed:?}" + ); + } + + /// The list is what the shell menu offers, so a distro that cannot open a + /// pane must not be on it: `wsl -l -q`, which this replaced, only ever + /// listed installed ones. + #[test] + fn a_distro_that_is_not_installed_is_not_offered() { + let Some(guids) = super::registry_user_subkeys(super::LXSS) else { + eprintln!("skipping: the registry has no distro list to read"); + return; + }; + let half_installed: Vec = guids + .iter() + .map(|guid| format!(r"{}\{guid}", super::LXSS)) + .filter(|key| super::registry_user_dword(key, "State").is_some_and(|state| state != 1)) + .filter_map(|key| super::registry_user_string(&key, "DistributionName")) + .collect(); + if half_installed.is_empty() { + eprintln!("skipping: every registered distro finished installing"); + return; + } + let offered = super::wsl_distros(); + for name in &half_installed { + assert!( + !offered.contains(name), + "unfinished distro {name:?} was offered in {offered:?}" + ); + } + } } pub fn wsl_distros() -> Vec { wsl_distros_probed().unwrap_or_default() } +/// Where `wsl.exe` registers what is installed: one subkey per distro, named by +/// GUID, carrying `DistributionName` and `State`. +#[cfg(windows)] +const LXSS: &str = r"Software\Microsoft\Windows\CurrentVersion\Lxss"; + /// The distro `wsl.exe` launches when no `--distribution` is given, read from /// the registry (`Lxss\DefaultDistribution` names the per-distro key that /// carries `DistributionName`). The registry rather than `wsl -l`: this runs /// on the pane-spawn path, where a microsecond read beats a subprocess. #[cfg(windows)] pub fn default_wsl_distro() -> Option { - const LXSS: &str = r"Software\Microsoft\Windows\CurrentVersion\Lxss"; let guid = registry_user_string(LXSS, "DefaultDistribution")?; let name = registry_user_string(&format!(r"{LXSS}\{guid}"), "DistributionName")?; (!name.is_empty()).then_some(name) @@ -568,8 +652,161 @@ fn find_git_bash() -> Option { #[cfg(windows)] const WSL_LIST_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(3); +/// Distros that exist to carry a container runtime, not to be typed into. +#[cfg_attr(unix, allow(dead_code))] +const NOT_FOR_TYPING: [&str; 2] = ["docker-desktop", "rancher-desktop"]; + +#[cfg_attr(unix, allow(dead_code))] +fn worth_offering(name: &str) -> bool { + !name.is_empty() && !NOT_FOR_TYPING.iter().any(|hidden| name.starts_with(hidden)) +} + +/// The installed distros, from `Lxss` — the same registry key `wsl.exe` itself +/// registers them in, and the one `default_wsl_distro` above already reads. +/// +/// Not `wsl -l -q`, because that has to reach the WSL service, and reaching the +/// WSL service is exactly the part that can be slow: issue #454 was a machine +/// where it took 3.3s, past the timeout below, so the list came back empty +/// every time and no distro was ever offered in the shell menu. A registry read +/// is microseconds and cannot hang, because nothing is listening on it. +/// +/// Windows Terminal made this same move in 2021 (microsoft/terminal#10967) for +/// the same reason, but skips distros whose key carries `Modern = 1`. That is a +/// deduplication rule specific to Terminal — modern distros ship it a profile +/// fragment of their own, so reading both would list them twice. Nothing ships +/// tty7 anything, so we take them all; skipping them here would hide the most +/// ordinary distro on an up-to-date machine. +/// +/// `State` we do read, the way Terminal does: a distro is only offered while it +/// says 1, "installed". An install that was interrupted — `wsl --install` shut +/// down halfway, a failed `--import`, one being uninstalled right now — leaves +/// the key behind with a name and some other state, and `wsl -l -q` (which this +/// replaced) never listed those. Offering one puts a distro in the shell menu +/// that can only open a pane that dies of a WSL registration error. +/// +/// `None` means "could not tell", never "there is nothing": the caller falls +/// back to `wsl.exe` on it, and a caller further up keeps the last good list. +#[cfg(windows)] +fn registered_wsl_distros() -> Option> { + let guids = registry_user_subkeys(LXSS)?; + let names: Vec = guids + .iter() + .map(|guid| format!(r"{LXSS}\{guid}")) + // A key with no `State` at all is taken at its word: the absent value + // is not evidence of a broken install, and inventing one would be how + // this hides a working distro. + .filter(|key| registry_user_dword(key, "State").unwrap_or(INSTALLED) == INSTALLED) + .filter_map(|key| registry_user_string(&key, "DistributionName")) + .filter(|name| worth_offering(name)) + .collect(); + + // Subkeys but nothing to show for them is not an answer either: every name + // unreadable has the shape of a permissions problem, not of a machine with + // no distros on it — that machine has an empty `Lxss`, and says so. + if names.is_empty() && !guids.is_empty() { + return None; + } + Some(names) +} + +/// `State` of a distro that finished installing and has not started leaving. +#[cfg(windows)] +const INSTALLED: u32 = 1; + +#[cfg(windows)] +fn registry_user_dword(subkey: &str, value: &str) -> Option { + use windows_sys::Win32::System::Registry::{HKEY_CURRENT_USER, RRF_RT_REG_DWORD, RegGetValueW}; + + fn wide(s: &str) -> Vec { + s.encode_utf16().chain(std::iter::once(0)).collect() + } + let (subkey, value) = (wide(subkey), wide(value)); + let mut data: u32 = 0; + let mut size = std::mem::size_of::() as u32; + // SAFETY: both names are NUL-terminated and owned here, and `data` is a + // live u32 exactly `size` bytes long, which is what a DWORD read writes. + let rc = unsafe { + RegGetValueW( + HKEY_CURRENT_USER, + subkey.as_ptr(), + value.as_ptr(), + RRF_RT_REG_DWORD, + std::ptr::null_mut(), + (&raw mut data).cast(), + &mut size, + ) + }; + (rc == 0).then_some(data) +} + +/// The names of a key's subkeys, or `None` if they could not all be read. +/// +/// All or nothing on purpose. The list this feeds is what the shell menu offers, +/// and a caller that cannot tell a short list from a complete one would quietly +/// drop distros: the walk is by index, so a key that changes underneath it — +/// `wsl --unregister` running right now, a Store install rewriting `Lxss` — +/// ends early, and reporting that as the answer is worse than admitting it. +#[cfg(windows)] +fn registry_user_subkeys(subkey: &str) -> Option> { + use windows_sys::Win32::Foundation::ERROR_NO_MORE_ITEMS; + use windows_sys::Win32::System::Registry::{ + HKEY, HKEY_CURRENT_USER, KEY_READ, RegCloseKey, RegEnumKeyExW, RegOpenKeyExW, + }; + + let subkey: Vec = subkey.encode_utf16().chain(std::iter::once(0)).collect(); + let mut key: HKEY = std::ptr::null_mut(); + // SAFETY: `subkey` is NUL-terminated and owned here; `key` is written only + // on success and closed on every path out below. + if unsafe { RegOpenKeyExW(HKEY_CURRENT_USER, subkey.as_ptr(), 0, KEY_READ, &mut key) } != 0 { + return None; + } + + let mut names = Vec::new(); + // A registry key name is at most 255 characters, plus the terminator. + let mut buf = [0u16; 256]; + let mut ended_with = None; + for index in 0.. { + let mut len = buf.len() as u32; + // SAFETY: `buf` really is `len` units long, and every pointer that is + // not wanted is null, which this call documents as "do not report it". + let rc = unsafe { + RegEnumKeyExW( + key, + index, + buf.as_mut_ptr(), + &mut len, + std::ptr::null_mut(), + std::ptr::null_mut(), + std::ptr::null_mut(), + std::ptr::null_mut(), + ) + }; + if rc != 0 { + ended_with = Some(rc); + break; + } + names.push(String::from_utf16_lossy(&buf[..len as usize])); + } + + // SAFETY: `key` was opened above and is not used after this. + unsafe { RegCloseKey(key) }; + // There is one honest way for the walk to end. Anything else — the key + // deleted underneath it, a name that would not fit — leaves a list that is + // short by an unknown amount, which nobody downstream can tell from a real + // one, so say nothing instead. + (ended_with == Some(ERROR_NO_MORE_ITEMS)).then_some(names) +} + #[cfg(windows)] fn list_wsl_distros() -> Option> { + if let Some(registered) = registered_wsl_distros() { + return Some(registered); + } + + // The registry would not answer — no `Lxss` key at all, or a walk of it that + // ended somewhere other than the end. Either way this is not a "there are no + // distros" to pass on, so ask the slow way rather than claim there is nothing. + log::debug!("{LXSS} gave no usable answer; falling back to `wsl -l -q`"); let mut cmd = std::process::Command::new("wsl.exe"); cmd.args(["-l", "-q"]); let output = match crate::core::proc::output_within( @@ -597,7 +834,7 @@ fn parse_wsl_list(bytes: &[u8]) -> Vec { let text = String::from_utf16_lossy(&units); text.lines() .map(|l| l.trim_matches(|c: char| c.is_whitespace() || c == '\u{feff}' || c == '\0')) - .filter(|l| !l.is_empty() && !l.starts_with("docker-desktop")) + .filter(|l| worth_offering(l)) .map(str::to_string) .collect() } diff --git a/crates/tty7-core/src/daemon/install/wsl.rs b/crates/tty7-core/src/daemon/install/wsl.rs index 9570272b..32bf96f1 100644 --- a/crates/tty7-core/src/daemon/install/wsl.rs +++ b/crates/tty7-core/src/daemon/install/wsl.rs @@ -565,10 +565,100 @@ fn install_lock(distro: &str) -> Arc> { lock } +/// Where a distro's server was last proved to be, so the next pane can skip the +/// proving. +/// +/// `Installer::run` costs five serial `wsl.exe` round trips — `uname`, `$HOME`, +/// a stat, a liveness probe, and a look at what is running. That is a fine price +/// to pay once for a distro, and an absurd one to pay per pane: issue #454 was a +/// machine where one round trip took 3.3s, so opening a second tab on a distro +/// that was already connected cost half a minute to re-learn what the first tab +/// had just learned. +/// +/// Nothing here expires on a timer, because the answer barely rots: +/// +/// - The binary does not move. A tty7 upgrade renames it, but a new build is a +/// new process and this map lives only in memory, so it starts empty. +/// - The distro shutting down does not invalidate it either. `wsl.exe` starts a +/// stopped distro on demand, and the bridge (`tty7-server --stdio --pane`) +/// starts its own daemon if none is listening — so the one thing that really +/// does stop being true, "a daemon is running in there", is repaired a layer +/// below us without anyone asking. +/// +/// What is left is a path that could stop existing: the distro reinstalled, the +/// binary deleted by hand. Only the bridge discovers that, and only once it is +/// running — so the router forgets the distro when a bridge dies without ever +/// answering, and the pane after that proves it again. See `forget_wsl_server`. +static READY: Mutex> = Mutex::new(Vec::new()); + +#[derive(Clone)] +struct Proved { + binary: String, + /// The build mismatch the probe found, if it found one. + /// + /// Kept because the warning is produced inside `Installer::run`, and the + /// whole point of the note is that `run` does not happen again: without + /// this, only the first pane of the daemon's lifetime would ever hear that + /// a different build is serving the distro, and every window opened after + /// it — including a whole new GUI session, since the daemon outlives one — + /// would attach in silence. + mismatch: Option, +} + +fn remembered(distro: &str) -> Option { + READY + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .iter() + .find(|(d, _)| d == distro) + .map(|(_, proved)| proved.clone()) +} + +fn remember(distro: &str, proved: Proved) { + let mut ready = READY + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + match ready.iter_mut().find(|(d, _)| d == distro) { + Some((_, known)) => *known = proved, + None => ready.push((distro.to_string(), proved)), + } +} + +/// Where this distro's server was last proved to be, or `None` if the next pane +/// would have to go and ask. A hint for callers deciding whether a failure is +/// worth re-proving; the answer itself comes from `ensure_wsl_server`. +pub fn remembered_wsl_server(distro: &str) -> Option { + remembered(distro).map(|proved| proved.binary) +} + +/// Drop what we thought we knew about a distro, so the next `ensure_wsl_server` +/// proves it again the long way. +pub fn forget_wsl_server(distro: &str) { + READY + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .retain(|(d, _)| d != distro); +} + pub fn ensure_wsl_server(distro: &str) -> io::Result { validate_distro(distro)?; + + // Under the lock even when the answer is only going to be read, because the + // note names a file that `replace_wsl_server` is in the business of moving: + // reading it outside would let a pane spawn the very binary a replace is + // deleting. The lock is uncontended except during an install, and waiting + // for an install to finish is what a pane wants to do anyway. let lock = install_lock(distro); let _held = lock.lock().unwrap_or_else(|poisoned| poisoned.into_inner()); + if let Some(proved) = remembered(distro) { + // Re-file the warning rather than re-run the probe that found it: this + // route's sink is a fresh one, and the client on the other end of it + // has not heard about the mismatch yet. + if let Some(mismatch) = proved.mismatch { + super::record_remote_mismatches(vec![mismatch]); + } + return Ok(proved.binary); + } let ops = WslRemoteOps::new(distro); let source = BundledServerBinary::discover(); @@ -595,6 +685,13 @@ pub fn ensure_wsl_server(distro: &str) -> io::Result { "" }, ); + remember( + distro, + Proved { + binary: report.paths.binary.clone(), + mismatch: report.mismatch, + }, + ); Ok(report.paths.binary) } @@ -605,6 +702,11 @@ pub fn restart_wsl_daemon(distro: &str) -> io::Result<()> { let confirm = install_confirm(); let lock = install_lock(distro); let _held = lock.lock().unwrap_or_else(|poisoned| poisoned.into_inner()); + // Both of these deliberately change what is running in there, which is the + // one thing the remembered answer is a claim about. Forget it first: if the + // restart fails halfway, the next pane must go and look rather than trust a + // note written before the upheaval. + forget_wsl_server(distro); Installer::with_source(&ops, &source, confirm.as_ref(), host_label(distro)).restart_daemon()?; Ok(()) } @@ -619,6 +721,7 @@ pub fn replace_wsl_server(distro: &str) -> io::Result<()> { let confirm = install_confirm(); let lock = install_lock(distro); let _held = lock.lock().unwrap_or_else(|poisoned| poisoned.into_inner()); + forget_wsl_server(distro); Installer::with_source(&ops, &source, confirm.as_ref(), host_label(distro)).replace()?; Ok(()) } @@ -1561,6 +1664,147 @@ mod tests { assert!(a2.try_lock().is_ok()); } + /// Names no real distribution can have, so these tests never touch one and + /// never collide with each other when the suite runs in parallel. + fn nowhere(test: &str) -> String { + format!("tty7-no-such-distro-{test}") + } + + fn note(binary: &str) -> Proved { + Proved { + binary: binary.to_string(), + mismatch: None, + } + } + + #[test] + fn a_remembered_distro_is_answered_without_asking_wsl_anything() { + let distro = nowhere("remembered"); + let binary = "/home/me/.local/share/tty7/bin/tty7-server-c5p5"; + remember(&distro, note(binary)); + + // There is no such distribution, so a probe could only have failed: + // getting the path back at all is what proves none ran. + let answered = ensure_wsl_server(&distro).expect("the note is the answer"); + assert_eq!(answered, binary); + + forget_wsl_server(&distro); + } + + #[test] + fn forgetting_sends_the_next_caller_back_to_the_distribution() { + let distro = nowhere("forgotten"); + remember(&distro, note("/somewhere/tty7-server")); + assert!(remembered_wsl_server(&distro).is_some()); + + forget_wsl_server(&distro); + assert_eq!(remembered_wsl_server(&distro), None); + assert!( + ensure_wsl_server(&distro).is_err(), + "a forgotten distro must be proved again, not assumed" + ); + } + + #[test] + fn what_is_remembered_is_per_distro_and_replaceable() { + let (a, b) = (nowhere("map-a"), nowhere("map-b")); + remember(&a, note("/a/tty7-server")); + remember(&b, note("/b/tty7-server")); + assert_eq!(remembered_wsl_server(&a).as_deref(), Some("/a/tty7-server")); + + forget_wsl_server(&a); + assert_eq!(remembered_wsl_server(&a), None); + assert_eq!( + remembered_wsl_server(&b).as_deref(), + Some("/b/tty7-server"), + "forgetting one distro must not forget another" + ); + + remember(&b, note("/b/tty7-server-newer")); + assert_eq!( + remembered_wsl_server(&b).as_deref(), + Some("/b/tty7-server-newer"), + "a later answer replaces the earlier one" + ); + forget_wsl_server(&b); + } + + #[test] + fn a_restart_forgets_first_so_a_failed_one_leaves_no_stale_note() { + let distro = nowhere("restart"); + remember(&distro, note("/x/tty7-server")); + + // This cannot succeed — there is no such distribution — which is the + // point: the note must be gone even though the work after it failed. + let _ = restart_wsl_daemon(&distro); + assert_eq!(remembered_wsl_server(&distro), None); + } + + #[test] + fn a_remembered_answer_waits_for_an_install_to_let_go_of_the_binary() { + let distro = nowhere("locked"); + remember(&distro, note("/x/tty7-server")); + + // Stand in for a `replace_wsl_server` in progress: it holds this lock + // while it moves the very file the note names. + let lock = install_lock(&distro); + let held = lock.lock().expect("a lock nobody else has"); + + let (tx, rx) = std::sync::mpsc::channel(); + let asking = { + let distro = distro.clone(); + std::thread::spawn(move || tx.send(ensure_wsl_server(&distro))) + }; + assert!( + rx.recv_timeout(Duration::from_millis(250)).is_err(), + "the note was handed out while a replace was under way" + ); + + drop(held); + let answered = rx + .recv_timeout(Duration::from_secs(5)) + .expect("answered once the install let go") + .expect("the note is the answer"); + assert_eq!(answered, "/x/tty7-server"); + let _ = asking.join(); + + forget_wsl_server(&distro); + } + + #[test] + fn a_remembered_mismatch_is_told_to_every_later_pane() { + let distro = nowhere("mismatch"); + let entry = crate::daemon::install::MismatchedRemoteDaemon { + host: host_label(&distro), + running_version: Some("0.0.1".to_string()), + running_exe: Some("/x/tty7-server-someone-elses".to_string()), + wanted_version: "9.9.9".to_string(), + }; + remember( + &distro, + Proved { + binary: "/x/tty7-server".to_string(), + mismatch: Some(entry.clone()), + }, + ); + + // A later pane is a fresh route with a fresh sink, and the client on + // the other end of it has never been told. + let sink = Arc::new(Mutex::new(Vec::new())); + let answered = + crate::daemon::install::with_mismatch_sink(sink.clone(), || ensure_wsl_server(&distro)) + .expect("the note is the answer"); + + assert_eq!(answered, "/x/tty7-server"); + assert_eq!( + &*sink.lock().expect("the sink"), + &[entry], + "the warning stopped at the first pane" + ); + + forget_wsl_server(&distro); + } + #[test] fn ensure_refuses_an_unusable_distro_name_before_spawning_anything() { let err = ensure_wsl_server("--shutdown").expect_err("refused"); diff --git a/crates/tty7-core/src/daemon/router.rs b/crates/tty7-core/src/daemon/router.rs index b414e8af..12d0bf13 100644 --- a/crates/tty7-core/src/daemon/router.rs +++ b/crates/tty7-core/src/daemon/router.rs @@ -605,7 +605,25 @@ async fn drive(local: Stream, header: &RouteHeader) -> io::Result<()> { if !leftover.is_empty() { tokio::io::AsyncWriteExt::write_all(&mut *link, &leftover).await?; } - let (to_remote, to_local) = tokio::io::copy_bidirectional(&mut local, &mut *link).await?; + let copied = tokio::io::copy_bidirectional(&mut local, &mut *link).await; + // A bridge that never sent a byte never ran. This is where a stale note is + // actually found out: `wsl.exe` spawns quite happily with a server path + // that no longer exists inside the distro — the distro was reinstalled, the + // directory was cleaned out — and only fails once it is the shell trying to + // exec it. Forget the distro, so the pane after this one proves it again + // rather than repeating a failure that would otherwise outlive every window + // and last until tty7 itself restarts. + if let RouteTarget::Wsl { distro } = &header.target + && header.server_command.is_none() + && !copied + .as_ref() + .is_ok_and(|(_, from_remote)| *from_remote > 0) + { + log::info!("wsl:{distro}: the bridge closed without answering; proving it again next time"); + crate::daemon::install::wsl::forget_wsl_server(distro); + } + + let (to_remote, to_local) = copied?; log::debug!("routed connection closed after {to_remote} up / {to_local} down bytes"); drop(conn); Ok(()) @@ -742,6 +760,15 @@ async fn restart_server( } } +/// Prove (or recall) where this distro's server is, off the reactor — the probe +/// is a chain of blocking `wsl.exe` calls the first time round. +async fn ensure_wsl_server(distro: &str, setup: &RouteSetup) -> anyhow::Result { + let distro = distro.to_string(); + Ok(setup + .blocking(move || crate::daemon::install::wsl::ensure_wsl_server(&distro)) + .await??) +} + async fn open_link( header: &RouteHeader, setup: &RouteSetup, @@ -754,25 +781,30 @@ async fn open_link( Ok((link, Some(conn))) } RouteTarget::Wsl { distro } => { - let resolved = match header.server_command { - Some(_) => None, - None => { - let distro = distro.clone(); - Some( - setup - .blocking(move || { - crate::daemon::install::wsl::ensure_wsl_server(&distro) - }) - .await??, - ) + if let Some(command) = header.server_command.as_deref() { + let link = RemoteLink::wsl_shell(distro, command, setup.channel)?; + return Ok((link, None)); + } + + let from_memory = crate::daemon::install::wsl::remembered_wsl_server(distro).is_some(); + let binary = ensure_wsl_server(distro, setup).await?; + match RemoteLink::wsl(distro, &binary, setup.channel) { + Ok(link) => Ok((link, None)), + // Only worth a second look when the path came from memory: one + // proved a moment ago will prove the same, and re-proving it + // just doubles the wait before the error reaches the user. + Err(stale) if from_memory => { + log::info!( + "wsl:{distro}: the remembered server would not start ({stale}); \ + looking again" + ); + crate::daemon::install::wsl::forget_wsl_server(distro); + let binary = ensure_wsl_server(distro, setup).await?; + let link = RemoteLink::wsl(distro, &binary, setup.channel)?; + Ok((link, None)) } - }; - let link = match (header.server_command.as_deref(), resolved.as_deref()) { - (Some(command), _) => RemoteLink::wsl_shell(distro, command, setup.channel)?, - (None, Some(binary)) => RemoteLink::wsl(distro, binary, setup.channel)?, - (None, None) => unreachable!("resolved is Some whenever there is no override"), - }; - Ok((link, None)) + Err(e) => Err(e.into()), + } } RouteTarget::LocalStdio { program, args } => { let args: Vec<&str> = args.iter().map(String::as_str).collect(); diff --git a/src/terminal/remote.rs b/src/terminal/remote.rs index 6ce9ca85..7b660143 100644 --- a/src/terminal/remote.rs +++ b/src/terminal/remote.rs @@ -2119,11 +2119,18 @@ fn connect_routed(route: &PaneRoute) -> anyhow::Result { tty7_core::host::guard_off_ui(); - if let crate::daemon::router::RouteTarget::Wsl { distro } = &header.target { - crate::daemon::install::wsl::ensure_wsl_server(distro) - .map_err(|e| anyhow::anyhow!("prepare tty7-server in WSL `{distro}`: {e}"))?; - } - + // No `ensure_wsl_server` here on purpose. The daemon runs exactly the same + // probe inside `router::open_link` before it opens the link, so asking from + // this side too bought nothing and cost a second full round of `wsl.exe` + // invocations — five of them, serially, on every single pane. On a machine + // where a `wsl.exe` round trip is slow (issue #454 measured 3.3s) that + // duplicate was half of the wait before a new tab could take a key. + // + // Nothing is lost by dropping it: the returned path was discarded, the + // failure is reported just as well through the route ack below, and the + // first-install consent question still reaches this process — the daemon + // runs its probe under `RouteSetup::blocking`, which installs the relay + // that turns the question into a frame on this very connection. let mut stream = connect()?; let ack = crate::daemon::router::negotiate(&mut stream, header) .map_err(|e| anyhow::anyhow!("route this pane to {}: {e}", header.describe()))?;