From f676fb96de1615d3098b8cfcde09e9a82d90843d Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Tue, 11 Aug 2026 09:36:51 +0800 Subject: [PATCH 1/5] perf(wsl): stop asking twice whether a distro is ready MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every pane on a WSL workspace ran `ensure_wsl_server` from the client before it even connected to the daemon — and then the daemon ran the very same probe inside `router::open_link` before opening the link. Two full rounds of five serial `wsl.exe` calls, to learn one fact. The client's copy bought nothing. It threw the answer away and kept only the error, which the route ack reports just as well; and the consent question for a first install still finds its way here, because the daemon runs its probe under `RouteSetup::blocking`, which installs the relay that turns that question into a frame on this connection. Measured on a distro that was already running and connected: 800ms to open a tab, down to 440ms. Issue #454 is the same code on a machine where one `wsl.exe` round trip takes 3.3s, where the duplicate was costing 15s a tab. --- src/terminal/remote.rs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/terminal/remote.rs b/src/terminal/remote.rs index 51ba3362..4d01b14b 100644 --- a/src/terminal/remote.rs +++ b/src/terminal/remote.rs @@ -2105,11 +2105,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()))?; From c5a0d8665b88ae05c2e3d626054a1cc8b5562243 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Tue, 11 Aug 2026 09:37:00 +0800 Subject: [PATCH 2/5] perf(wsl): let a distro say once where its server is MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ensure_wsl_server` re-proved everything on every pane: uname, $HOME, a stat, a liveness probe, a look at what is running — five serial `wsl.exe` round trips to re-learn what the previous pane had just learned. Fine once per distro, absurd per pane. It now keeps the answer in memory, and nothing expires on a timer, because the answer barely rots. A tty7 upgrade renames the binary, but a new build is a new process and the map starts empty. A distro shutting down does not invalidate it either: `wsl.exe` restarts a stopped distro on demand, and the bridge starts its own daemon when none is listening, so the one claim that really does stop being true is repaired a layer below without anyone asking. What is left is a path that could stop existing — the distro reinstalled, the directory cleaned out. Starting the bridge is what discovers that, so the router forgets the distro and proves it again from scratch, once. The two operations that deliberately disturb what is running forget first, so a restart that fails halfway leaves no note claiming otherwise. --- crates/tty7-core/src/daemon/install/wsl.rs | 146 +++++++++++++++++++++ crates/tty7-core/src/daemon/router.rs | 52 +++++--- 2 files changed, 180 insertions(+), 18 deletions(-) diff --git a/crates/tty7-core/src/daemon/install/wsl.rs b/crates/tty7-core/src/daemon/install/wsl.rs index 9570272b..93e0c75e 100644 --- a/crates/tty7-core/src/daemon/install/wsl.rs +++ b/crates/tty7-core/src/daemon/install/wsl.rs @@ -565,10 +565,73 @@ 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. Spawning the bridge is what discovers that, so the +/// router forgets the distro when the bridge will not start and proves it again +/// from scratch. See `forget_wsl_server`. +static READY: Mutex> = Mutex::new(Vec::new()); + +fn remembered(distro: &str) -> Option { + READY + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .iter() + .find(|(d, _)| d == distro) + .map(|(_, binary)| binary.clone()) +} + +fn remember(distro: &str, binary: &str) { + let mut ready = READY + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + match ready.iter_mut().find(|(d, _)| d == distro) { + Some((_, known)) => *known = binary.to_string(), + None => ready.push((distro.to_string(), binary.to_string())), + } +} + +/// 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)?; + if let Some(binary) = remembered(distro) { + return Ok(binary); + } + let lock = install_lock(distro); let _held = lock.lock().unwrap_or_else(|poisoned| poisoned.into_inner()); + // Someone may have proved it while this thread queued for the lock — which + // is exactly what happens when a window restores several panes at once. + if let Some(binary) = remembered(distro) { + return Ok(binary); + } let ops = WslRemoteOps::new(distro); let source = BundledServerBinary::discover(); @@ -595,6 +658,7 @@ pub fn ensure_wsl_server(distro: &str) -> io::Result { "" }, ); + remember(distro, &report.paths.binary); Ok(report.paths.binary) } @@ -605,6 +669,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 +688,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 +1631,82 @@ 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}") + } + + #[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, binary); + + // There is no such distribution, so an answer at all proves the probe + // was skipped — and a fast one proves it twice over. + let started = std::time::Instant::now(); + let answered = ensure_wsl_server(&distro).expect("the note is the answer"); + let elapsed = started.elapsed(); + + assert_eq!(answered, binary); + assert!( + elapsed < Duration::from_millis(200), + "the probe ran anyway: {elapsed:?}" + ); + + forget_wsl_server(&distro); + } + + #[test] + fn forgetting_sends_the_next_caller_back_to_the_distribution() { + let distro = nowhere("forgotten"); + remember(&distro, "/somewhere/tty7-server"); + assert!(remembered(&distro).is_some()); + + forget_wsl_server(&distro); + assert_eq!(remembered(&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, "/a/tty7-server"); + remember(&b, "/b/tty7-server"); + assert_eq!(remembered(&a).as_deref(), Some("/a/tty7-server")); + + forget_wsl_server(&a); + assert_eq!(remembered(&a), None); + assert_eq!( + remembered(&b).as_deref(), + Some("/b/tty7-server"), + "forgetting one distro must not forget another" + ); + + remember(&b, "/b/tty7-server-newer"); + assert_eq!( + remembered(&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, "/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(&distro), None); + } + #[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..0e712439 100644 --- a/crates/tty7-core/src/daemon/router.rs +++ b/crates/tty7-core/src/daemon/router.rs @@ -742,6 +742,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 +763,32 @@ 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 binary = ensure_wsl_server(distro, setup).await?; + match RemoteLink::wsl(distro, &binary, setup.channel) { + Ok(link) => Ok((link, None)), + // `ensure_wsl_server` answers from memory after the first pane, + // and the note it kept can be wrong in exactly one way: the + // binary is no longer at that path (the distro was reinstalled, + // someone cleaned the directory out). Starting the bridge is + // what finds out, so pay the full probe once more rather than + // fail a pane over a stale path — but only once, or a distro + // that genuinely cannot run it would loop. + Err(stale) => { + 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)) + } } RouteTarget::LocalStdio { program, args } => { let args: Vec<&str> = args.iter().map(String::as_str).collect(); From 7f16f6a6ff9a93d1527505c7b39f9cf4c874ed27 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Tue, 11 Aug 2026 09:40:37 +0800 Subject: [PATCH 3/5] fix(wsl): read the distro list from the registry, not the WSL service MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `wsl -l -q` has to reach the WSL service, and reaching the WSL service is the part that can be slow. Behind a hardcoded 3s timeout that made the listing all-or-nothing: on the machine in #454 a round trip took 3.3s, so the call timed out every time, the list came back empty every time, and no WSL distro was ever offered in the shell menu. Not slow — absent. `Lxss` is where `wsl.exe` registers them, it is the same key `default_wsl_distro` already reads for the same stated reason, and nothing is listening on it, so it cannot hang. `wsl -l -q` stays as the fallback for when the key will not open at all, which means this is not a machine with WSL on it rather than a machine whose WSL is busy. Windows Terminal made this move in 2021 (microsoft/terminal#10967) after the same symptom — distros "missing entirely" on first launch. It skips distros whose key carries `Modern = 1`; we must not. That is a deduplication rule specific to Terminal, which modern distros hand a profile fragment of their own. Nothing hands tty7 anything, and on an up-to-date machine `Modern = 1` is the ordinary case — on the box this was written on, the only distro installed. There is a test pinning that. --- crates/tty7-core/src/core/shells.rs | 165 +++++++++++++++++++++++++++- 1 file changed, 163 insertions(+), 2 deletions(-) diff --git a/crates/tty7-core/src/core/shells.rs b/crates/tty7-core/src/core/shells.rs index 8fa55afb..19f2e9b5 100644 --- a/crates/tty7-core/src/core/shells.rs +++ b/crates/tty7-core/src/core/shells.rs @@ -463,6 +463,51 @@ 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() { + 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:?}" + ); + } } pub fn wsl_distros() -> Vec { @@ -473,9 +518,11 @@ pub fn wsl_distros() -> Vec { /// 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)] +const LXSS: &str = r"Software\Microsoft\Windows\CurrentVersion\Lxss"; + #[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 +615,122 @@ 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. +#[cfg(windows)] +fn registered_wsl_distros() -> Option> { + let names = registry_user_subkeys(LXSS)? + .iter() + .filter_map(|guid| registry_user_string(&format!(r"{LXSS}\{guid}"), "DistributionName")) + .filter(|name| worth_offering(name)) + .collect(); + Some(names) +} + +#[cfg(all(windows, test))] +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) +} + +#[cfg(windows)] +fn registry_user_subkeys(subkey: &str) -> Option> { + 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]; + 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 { + // Anything other than success ends the walk — ERROR_NO_MORE_ITEMS + // in the ordinary case, and for a key that changed underneath us, + // stopping early beats reporting a half-read list as an error. + 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) }; + Some(names) +} + #[cfg(windows)] fn list_wsl_distros() -> Option> { + if let Some(registered) = registered_wsl_distros() { + return Some(registered); + } + + // The key would not open, so this is either not a machine with WSL on it or + // something stranger. Ask the slow way rather than claim there is nothing. + log::debug!("no {LXSS} key; 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 +758,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() } From e0745329db097cf64db1a58a7bde98898589b0e7 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Tue, 11 Aug 2026 10:25:30 +0800 Subject: [PATCH 4/5] fix(wsl): do not pass off a half-read registry as the distro list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `registry_user_subkeys` ended its walk on any non-zero return and reported what it had as the answer. Only one of those returns means "that was all of them"; the rest mean the walk stopped early — a `wsl --unregister` running right now, a Store install rewriting `Lxss` underneath it — and a failure at the very first index came back as `Some(vec![])`, an authoritative "there are no distros". The sweep that feeds the shell menu keeps the last good list only when the probe says `None`, so that empty answer erased the user's distros for the length of the TTL, with no error anywhere and no `wsl -l -q` to catch it: the fallback only runs when the key will not open at all. The walk now says nothing unless it reached the end. `State` is now read too, the way Windows Terminal reads it. A `DistributionName` is not a promise that the distro can be entered: an install that was cancelled half way, a failed `--import`, one being uninstalled as we look, all leave the key behind. `wsl -l -q`, which this replaced, never listed those; without the filter they arrive in the shell menu and open a pane that dies of a WSL registration error. A key with no `State` at all is still taken at its word, which is the conservative direction — inventing one would hide working distros, which is the mistake `Modern = 1` would have been. That also makes `registry_user_dword` production code rather than a `cfg(test)` copy of `registry_user_string`'s FFI scaffolding kept alive for one assertion. The timing test now skips when the registry has nothing to read: on a machine with no `Lxss` key the listing is *supposed* to go to `wsl.exe` and wait, so timing it there failed the test on exactly the machines the fallback is for. And hoisting `LXSS` had left `default_wsl_distro`'s doc comment attached to the const; it goes back on the function. --- crates/tty7-core/src/core/shells.rs | 102 ++++++++++++++++++++++++---- 1 file changed, 89 insertions(+), 13 deletions(-) diff --git a/crates/tty7-core/src/core/shells.rs b/crates/tty7-core/src/core/shells.rs index 19f2e9b5..32ffba12 100644 --- a/crates/tty7-core/src/core/shells.rs +++ b/crates/tty7-core/src/core/shells.rs @@ -500,6 +500,13 @@ mod wsl_tests { #[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(); @@ -508,19 +515,49 @@ mod wsl_tests { "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)] -const LXSS: &str = r"Software\Microsoft\Windows\CurrentVersion\Lxss"; - #[cfg(windows)] pub fn default_wsl_distro() -> Option { let guid = registry_user_string(LXSS, "DefaultDistribution")?; @@ -639,17 +676,44 @@ fn worth_offering(name: &str) -> bool { /// 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 names = registry_user_subkeys(LXSS)? + let guids = registry_user_subkeys(LXSS)?; + let names: Vec = guids .iter() - .filter_map(|guid| registry_user_string(&format!(r"{LXSS}\{guid}"), "DistributionName")) + .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) } -#[cfg(all(windows, test))] +/// `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}; @@ -675,8 +739,16 @@ fn registry_user_dword(subkey: &str, value: &str) -> Option { (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, }; @@ -692,6 +764,7 @@ fn registry_user_subkeys(subkey: &str) -> Option> { 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 @@ -709,9 +782,7 @@ fn registry_user_subkeys(subkey: &str) -> Option> { ) }; if rc != 0 { - // Anything other than success ends the walk — ERROR_NO_MORE_ITEMS - // in the ordinary case, and for a key that changed underneath us, - // stopping early beats reporting a half-read list as an error. + ended_with = Some(rc); break; } names.push(String::from_utf16_lossy(&buf[..len as usize])); @@ -719,7 +790,11 @@ fn registry_user_subkeys(subkey: &str) -> Option> { // SAFETY: `key` was opened above and is not used after this. unsafe { RegCloseKey(key) }; - Some(names) + // 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)] @@ -728,9 +803,10 @@ fn list_wsl_distros() -> Option> { return Some(registered); } - // The key would not open, so this is either not a machine with WSL on it or - // something stranger. Ask the slow way rather than claim there is nothing. - log::debug!("no {LXSS} key; falling back to `wsl -l -q`"); + // 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( From d09fc1087874d2858495957be3870b7e8d07ed0d Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Tue, 11 Aug 2026 10:25:43 +0800 Subject: [PATCH 5/5] fix(wsl): make the remembered server path safe to trust MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The note the last commit introduced had no working way to be wrong. Its only repair was the router forgetting the distro when `RemoteLink::wsl` returned an error, and that call only spawns `wsl.exe` — which starts perfectly happily with a server path that no longer exists inside the distro. The exec failure arrives later, as an EOF on the bridge, so a distro that was reinstalled or had its bin directory cleaned out failed every WSL tab from then on, with nothing re-installing it and no way out but restarting tty7. So forget it where the truth actually shows up: a bridge that closed without ever sending a byte never ran, and after one of those the next pane proves the distro again. The spawn-error retry stays, but only when the path came from memory — a path proved a moment ago will prove the same, and re-probing it just doubles the wait before the error reaches the user. Two more things the note quietly took away. It was read before `install_lock`, so a pane spawn was no longer mutually exclusive with `replace_wsl_server`, whose whole job is to move the file the note names: a window restoring panes while the user updates the WSL server could spawn the binary being replaced. The read moves under the lock, which costs nothing when no install is running and correctly waits when one is. And it short-circuited `Installer::run`, the only thing that notices a foreign build serving the distro — so the "a different build of tty7-server is serving this machine" warning reached the first pane of the daemon's lifetime and no other, including a whole new GUI session, since the daemon outlives one. The note now carries the mismatch it found and re-files it for each later pane, which is the same warning without the five round trips that found it. The wall-clock budget in the remembered-answer test is gone: the returned path already proves no probe ran, and 200ms of elapsed time on a loaded CI box only ever proved the box was loaded. --- crates/tty7-core/src/daemon/install/wsl.rs | 176 ++++++++++++++++----- crates/tty7-core/src/daemon/router.rs | 34 ++-- 2 files changed, 162 insertions(+), 48 deletions(-) diff --git a/crates/tty7-core/src/daemon/install/wsl.rs b/crates/tty7-core/src/daemon/install/wsl.rs index 93e0c75e..32bf96f1 100644 --- a/crates/tty7-core/src/daemon/install/wsl.rs +++ b/crates/tty7-core/src/daemon/install/wsl.rs @@ -586,30 +586,51 @@ fn install_lock(distro: &str) -> Arc> { /// below us without anyone asking. /// /// What is left is a path that could stop existing: the distro reinstalled, the -/// binary deleted by hand. Spawning the bridge is what discovers that, so the -/// router forgets the distro when the bridge will not start and proves it again -/// from scratch. See `forget_wsl_server`. -static READY: Mutex> = Mutex::new(Vec::new()); +/// 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()); -fn remembered(distro: &str) -> Option { +#[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(|(_, binary)| binary.clone()) + .map(|(_, proved)| proved.clone()) } -fn remember(distro: &str, binary: &str) { +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 = binary.to_string(), - None => ready.push((distro.to_string(), binary.to_string())), + 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) { @@ -621,16 +642,22 @@ pub fn forget_wsl_server(distro: &str) { pub fn ensure_wsl_server(distro: &str) -> io::Result { validate_distro(distro)?; - if let Some(binary) = remembered(distro) { - return Ok(binary); - } + // 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()); - // Someone may have proved it while this thread queued for the lock — which - // is exactly what happens when a window restores several panes at once. - if let Some(binary) = remembered(distro) { - return Ok(binary); + 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); @@ -658,7 +685,13 @@ pub fn ensure_wsl_server(distro: &str) -> io::Result { "" }, ); - remember(distro, &report.paths.binary); + remember( + distro, + Proved { + binary: report.paths.binary.clone(), + mismatch: report.mismatch, + }, + ); Ok(report.paths.binary) } @@ -1637,23 +1670,23 @@ mod tests { 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, binary); + remember(&distro, note(binary)); - // There is no such distribution, so an answer at all proves the probe - // was skipped — and a fast one proves it twice over. - let started = std::time::Instant::now(); + // 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"); - let elapsed = started.elapsed(); - assert_eq!(answered, binary); - assert!( - elapsed < Duration::from_millis(200), - "the probe ran anyway: {elapsed:?}" - ); forget_wsl_server(&distro); } @@ -1661,11 +1694,11 @@ mod tests { #[test] fn forgetting_sends_the_next_caller_back_to_the_distribution() { let distro = nowhere("forgotten"); - remember(&distro, "/somewhere/tty7-server"); - assert!(remembered(&distro).is_some()); + remember(&distro, note("/somewhere/tty7-server")); + assert!(remembered_wsl_server(&distro).is_some()); forget_wsl_server(&distro); - assert_eq!(remembered(&distro), None); + assert_eq!(remembered_wsl_server(&distro), None); assert!( ensure_wsl_server(&distro).is_err(), "a forgotten distro must be proved again, not assumed" @@ -1675,21 +1708,21 @@ mod tests { #[test] fn what_is_remembered_is_per_distro_and_replaceable() { let (a, b) = (nowhere("map-a"), nowhere("map-b")); - remember(&a, "/a/tty7-server"); - remember(&b, "/b/tty7-server"); - assert_eq!(remembered(&a).as_deref(), Some("/a/tty7-server")); + 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(&a), None); + assert_eq!(remembered_wsl_server(&a), None); assert_eq!( - remembered(&b).as_deref(), + remembered_wsl_server(&b).as_deref(), Some("/b/tty7-server"), "forgetting one distro must not forget another" ); - remember(&b, "/b/tty7-server-newer"); + remember(&b, note("/b/tty7-server-newer")); assert_eq!( - remembered(&b).as_deref(), + remembered_wsl_server(&b).as_deref(), Some("/b/tty7-server-newer"), "a later answer replaces the earlier one" ); @@ -1699,12 +1732,77 @@ mod tests { #[test] fn a_restart_forgets_first_so_a_failed_one_leaves_no_stale_note() { let distro = nowhere("restart"); - remember(&distro, "/x/tty7-server"); + 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(&distro), None); + 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] diff --git a/crates/tty7-core/src/daemon/router.rs b/crates/tty7-core/src/daemon/router.rs index 0e712439..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(()) @@ -768,17 +786,14 @@ async fn open_link( 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)), - // `ensure_wsl_server` answers from memory after the first pane, - // and the note it kept can be wrong in exactly one way: the - // binary is no longer at that path (the distro was reinstalled, - // someone cleaned the directory out). Starting the bridge is - // what finds out, so pay the full probe once more rather than - // fail a pane over a stale path — but only once, or a distro - // that genuinely cannot run it would loop. - Err(stale) => { + // 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" @@ -788,6 +803,7 @@ async fn open_link( let link = RemoteLink::wsl(distro, &binary, setup.channel)?; Ok((link, None)) } + Err(e) => Err(e.into()), } } RouteTarget::LocalStdio { program, args } => {