From 6c3794addb236cb1f25db0f5f4b97d0ca5cc232b Mon Sep 17 00:00:00 2001 From: vincentzzh424 <146338078-vincentzzh424@users.noreply.github.com> Date: Tue, 30 Jun 2026 10:11:15 +0800 Subject: [PATCH 1/3] fix(terminal): stop dropped/duplicated chars on macOS fast typing macOS WKWebView tags ordinary fast typing with keyCode 229 ("Process"), not just IME. xterm.js skips its keydown->onData path for keyCode 229 and reads chars from textarea input events, which the browser coalesces under fast typing -- so some keystrokes never emit onData (chars dropped, e.g. cdcdcd came out ccccc) and at slow speed both our self-send and xterm's onData fire (chars doubled, e.g. c came out cc). Self-send keyCode-229 single-char keystrokes directly to the PTY and dedup against xterm's onData with a short window in both orderings, so exactly one copy of each char is sent. Real IME composition (isComposing) still goes through xterm's compositionend path unchanged. Also move pty_write off the webview main thread (async + spawn_blocking) so a burst of keystrokes can't serialize writes there. Co-Authored-By: Claude --- src-tauri/src/modules/pty/mod.rs | 39 +++++++------ src/modules/terminal/lib/rendererPool.ts | 73 +++++++++++++++++++++--- 2 files changed, 89 insertions(+), 23 deletions(-) diff --git a/src-tauri/src/modules/pty/mod.rs b/src-tauri/src/modules/pty/mod.rs index f38504e31..e99a2396c 100644 --- a/src-tauri/src/modules/pty/mod.rs +++ b/src-tauri/src/modules/pty/mod.rs @@ -94,10 +94,12 @@ pub async fn pty_open( // Input is the latency-critical path: raw body + id header skips JSON // serialization of every keystroke on both sides of the IPC boundary. +// Async + spawn_blocking so the write runs off the webview main thread; +// a burst of keystrokes under a sync command would stall that thread. #[tauri::command] -pub fn pty_write( - state: tauri::State, - request: tauri::ipc::Request, +pub async fn pty_write( + state: tauri::State<'_, PtyState>, + request: tauri::ipc::Request<'_>, ) -> Result<(), String> { let id: u32 = request .headers() @@ -108,6 +110,7 @@ pub fn pty_write( let tauri::ipc::InvokeBody::Raw(bytes) = request.body() else { return Err("pty_write: expected raw body".to_string()); }; + let bytes = bytes.clone(); let session = state .sessions .read() @@ -118,19 +121,23 @@ pub fn pty_write( log::warn!("pty_write: unknown id={id}"); "no session".to_string() })?; - // Bind to a local so the MutexGuard temporary drops before `session` — - // see rustc note on tail-expression temporary drop order. - let result = session - .writer - .lock() - .unwrap() - .write_all(bytes) - .map_err(|e| { - // EPIPE is expected if the child already exited. - log::debug!("pty_write id={id} failed: {e}"); - e.to_string() - }); - result + tauri::async_runtime::spawn_blocking(move || { + // EPIPE is expected if the child already exited. + session + .writer + .lock() + .unwrap() + .write_all(&bytes) + .map_err(|e| { + log::debug!("pty_write id={id} failed: {e}"); + e.to_string() + }) + }) + .await + .map_err(|e| { + log::error!("pty_write join failed: {e}"); + e.to_string() + })? } #[tauri::command] diff --git a/src/modules/terminal/lib/rendererPool.ts b/src/modules/terminal/lib/rendererPool.ts index 81c7d098a..bbdc78dec 100644 --- a/src/modules/terminal/lib/rendererPool.ts +++ b/src/modules/terminal/lib/rendererPool.ts @@ -70,6 +70,15 @@ export type Slot = { lastW: number; lastH: number; lastUsedAt: number; + // Dedup window for self-sent keyCode-229 chars vs xterm's own onData + // (see attachCustomKeyEventHandler): macOS emits both our self-send and a + // textarea-input onData for the same keystroke, so without coordination + // each keystroke doubles. The keystroke's keydown(kc=229) and textarea + // input(onData) can arrive in either order, so we dedup both directions. + lastOdChar: string | null; + lastOdAt: number; + selfSentChar: string | null; + selfSentAt: number; }; const slots: Slot[] = []; @@ -237,17 +246,52 @@ function createSlot(): Slot { lastW: 0, lastH: 0, lastUsedAt: 0, + lastOdChar: null, + lastOdAt: 0, + selfSentChar: null, + selfSentAt: 0, }; term.attachCustomKeyEventHandler((event) => { // During IME composition the browser is assembling a multi-keystroke - // character (Chinese pinyin → hanzi, Korean jamo → syllable, etc.). - // Raw keydown events — including the Enter that commits a candidate — - // must NOT be forwarded to the PTY; xterm will receive the final - // composed string through its own compositionend handler instead. - // keyCode 229 ("Process") is what Chromium reports for every key - // pressed inside an active IME session when isComposing is not yet set. - if (event.isComposing || event.keyCode === 229) return false; + // character (Chinese pinyin -> hanzi, Korean jamo -> syllable, etc.). + // Raw keydown events -- including the Enter that commits a candidate -- + // must NOT be forwarded to the PTY; xterm receives the final composed + // string through its own compositionend handler instead. + if (event.isComposing) return false; + + // macOS WKWebView tags ordinary fast typing as keyCode 229; xterm then + // reads chars from textarea input events, which coalesce under fast typing + // and drop keys. Forward kc=229 chars ourselves and dedup with onData. + if ( + event.type === "keydown" && + event.keyCode === 229 && + event.key.length === 1 + ) { + const leafId229 = slot.currentLeafId; + if (leafId229 !== null) { + const bridge229 = adapter?.resolveLeaf(leafId229); + if (bridge229) { + // xterm's textarea-input onData may have already fired for this + // keystroke just before keydown; if so, the char is already on its + // way and self-sending would double it. Otherwise self-send and mark + // so a trailing onData is dropped. + if ( + slot.lastOdChar === event.key && + Date.now() - slot.lastOdAt < 200 + ) { + slot.lastOdChar = null; + event.preventDefault(); + return false; + } + slot.selfSentChar = event.key; + slot.selfSentAt = Date.now(); + bridge229.writeToPty(event.key); + event.preventDefault(); + return false; + } + } + } const leafId = slot.currentLeafId; if (leafId === null) return false; @@ -301,6 +345,21 @@ function createSlot(): Slot { term.onData((data) => { const leafId = slot.currentLeafId; + // Drop xterm's own onData when it duplicates a char we already self-sent + // for a keyCode-229 keystroke within the dedup window. macOS fires both, + // so without this the self-send + onData would double the character. + if ( + slot.selfSentChar !== null && + data === slot.selfSentChar && + Date.now() - slot.selfSentAt < 200 + ) { + slot.selfSentChar = null; + return; + } + // Record this onData so a trailing keydown(kc=229) for the same char + // knows not to self-send ( onData-arrived-first ordering ). + slot.lastOdChar = data.length === 1 ? data : null; + slot.lastOdAt = Date.now(); if (leafId === null) return; adapter?.resolveLeaf(leafId)?.writeToPty(data); }); From 7fc3deb3410b9a3baed7fba9cda75efe5c78e894 Mon Sep 17 00:00:00 2001 From: vincentzzh424 <146338078-vincentzzh424@users.noreply.github.com> Date: Wed, 1 Jul 2026 10:29:49 +0800 Subject: [PATCH 2/3] =?UTF-8?q?fix(terminal):=20address=20review=20?= =?UTF-8?q?=E2=80=94=20FIFO=20kc=3D229=20dedup=20&=20ordered=20PTY=20write?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two correctness issues flagged by review on #904: 1. keyCode-229 dedup used a single scalar slot (selfSentChar/lastOdChar), so a fast burst like `cd` overwrote the pending char before its trailing onData arrived, leaking the older char through and doubling it (cd -> ccd). Replace with per-direction FIFO queues of {char, at} and consume the oldest matching entry within the 200ms window. 2. Each pty_write invoke spawned an independent spawn_blocking job that contended for session.writer.lock() with no ordering guarantee, so a burst of keystrokes could reach the PTY out of order and scramble input. Route all writes through a per-session mpsc channel drained by a single dedicated writer thread; pty_write now does a non-blocking send. Verified: cargo clippy --locked (clean), cargo test --locked (220 passed), pnpm check-types (clean), biome lint (0 new errors). Co-Authored-By: Claude --- src-tauri/src/modules/pty/mod.rs | 27 ++----- src-tauri/src/modules/pty/session.rs | 44 +++++++++--- src/modules/terminal/lib/rendererPool.ts | 91 ++++++++++++++---------- 3 files changed, 97 insertions(+), 65 deletions(-) diff --git a/src-tauri/src/modules/pty/mod.rs b/src-tauri/src/modules/pty/mod.rs index e99a2396c..33b7f97d6 100644 --- a/src-tauri/src/modules/pty/mod.rs +++ b/src-tauri/src/modules/pty/mod.rs @@ -4,7 +4,6 @@ mod session; pub(crate) mod shell_init; use std::collections::HashMap; -use std::io::Write; use std::sync::atomic::{AtomicU32, Ordering}; use std::sync::{Arc, RwLock}; use std::thread; @@ -93,9 +92,10 @@ pub async fn pty_open( } // Input is the latency-critical path: raw body + id header skips JSON -// serialization of every keystroke on both sides of the IPC boundary. -// Async + spawn_blocking so the write runs off the webview main thread; -// a burst of keystrokes under a sync command would stall that thread. +// serialization of every keystroke on both sides of the IPC boundary. Bytes +// are handed to the session's writer thread via an unbounded channel, so +// `send` is non-blocking and arrival order is preserved across a burst of +// keystrokes (the writer thread serializes the actual write_all calls). #[tauri::command] pub async fn pty_write( state: tauri::State<'_, PtyState>, @@ -121,23 +121,10 @@ pub async fn pty_write( log::warn!("pty_write: unknown id={id}"); "no session".to_string() })?; - tauri::async_runtime::spawn_blocking(move || { - // EPIPE is expected if the child already exited. - session - .writer - .lock() - .unwrap() - .write_all(&bytes) - .map_err(|e| { - log::debug!("pty_write id={id} failed: {e}"); - e.to_string() - }) + session.write_tx.send(bytes).map_err(|_| { + log::debug!("pty_write id={id}: writer channel closed"); + "no session".to_string() }) - .await - .map_err(|e| { - log::error!("pty_write join failed: {e}"); - e.to_string() - })? } #[tauri::command] diff --git a/src-tauri/src/modules/pty/session.rs b/src-tauri/src/modules/pty/session.rs index 792a98844..8f3f649cc 100644 --- a/src-tauri/src/modules/pty/session.rs +++ b/src-tauri/src/modules/pty/session.rs @@ -1,5 +1,6 @@ use std::io::{Read, Write}; use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::mpsc::{self, Sender}; use std::sync::{Arc, Condvar, Mutex}; use std::thread; use std::time::{Duration, Instant}; @@ -39,7 +40,10 @@ pub struct Session { // the Tauri worker thread that triggered the close. // 2. `killer` — best-effort kill (redundant on Windows once Job // closed, but harmless and required on Unix where there is no Job). - // 3. `writer` — closes the input side of the master pipe. + // 3. `write_tx` — drops the last channel sender, so the dedicated + // writer thread drains and exits, releasing the input-side writer + // clone it holds. (The reader thread's writer clone, used for DA + // replies, is released when the killed child's EOF unwinds it.) // 4. `master` — last; ClosePseudoConsole on Windows. By now the child // is dead and conhost has nothing left to drain. #[cfg(windows)] @@ -47,7 +51,10 @@ pub struct Session { /// PID of the shell process. 0 means unknown; callers must skip checks when 0. pub shell_pid: u32, pub killer: Mutex>, - pub writer: Arc>>, + // Input bytes are queued here and drained by a single dedicated writer + // thread, so a burst of pty_write calls stays in arrival order rather than + // racing for the writer lock from independent spawn_blocking jobs. + pub write_tx: Sender>, pub master: Mutex>, // Set by the waiter once the child exits, so pty_open can reap a shell // that died before it was registered. @@ -137,6 +144,25 @@ pub fn spawn( )); guard.disarm(); + // Dedicated writer thread: serializes all input writes in arrival order + // so concurrent pty_write calls can't reach `writer.lock()` out of order + // and scramble a fast burst of keystrokes. Unbounded channel => send never + // blocks the IPC caller; the thread exits when the last sender drops. + let (write_tx, write_rx) = mpsc::channel::>(); + let writer_for_writer = writer.clone(); + thread::Builder::new() + .name("terax-pty-writer".into()) + .spawn(move || { + for bytes in write_rx { + if let Ok(mut w) = writer_for_writer.lock() { + if let Err(e) = w.write_all(&bytes) { + log::debug!("pty writer thread write failed: {e}"); + } + } + } + }) + .expect("spawn pty writer thread"); + let shell_pid = child.process_id().unwrap_or(0); #[cfg(windows)] @@ -158,7 +184,7 @@ pub fn spawn( _job: job, shell_pid, killer: Mutex::new(killer), - writer: writer.clone(), + write_tx, master: Mutex::new(pair.master), exited: exited.clone(), }); @@ -337,13 +363,14 @@ mod tests { drop(pair.slave); let killer = child.clone_killer(); - let writer: Arc>> = - Arc::new(Mutex::new(pair.master.take_writer().expect("writer"))); + // take_writer so the writer side is owned and dropped with the test, + // but the Session no longer stores it directly. + drop(pair.master.take_writer().expect("writer")); let session = Arc::new(Session { shell_pid: child.process_id().unwrap_or(0), killer: Mutex::new(killer), - writer, + write_tx: mpsc::channel().0, master: Mutex::new(pair.master), exited: Arc::new(AtomicBool::new(false)), }); @@ -386,13 +413,12 @@ mod tests { let _ = child.wait(); let killer = child.clone_killer(); - let writer: Arc>> = - Arc::new(Mutex::new(pair.master.take_writer().expect("writer"))); + drop(pair.master.take_writer().expect("writer")); let session = Arc::new(Session { shell_pid: 0, killer: Mutex::new(killer), - writer, + write_tx: mpsc::channel().0, master: Mutex::new(pair.master), exited: Arc::new(AtomicBool::new(false)), }); diff --git a/src/modules/terminal/lib/rendererPool.ts b/src/modules/terminal/lib/rendererPool.ts index bbdc78dec..99adc6d29 100644 --- a/src/modules/terminal/lib/rendererPool.ts +++ b/src/modules/terminal/lib/rendererPool.ts @@ -24,6 +24,19 @@ const FIT_DEBOUNCE_MS = 8; const PTY_RESIZE_DEBOUNCE_MS = 256; const SNAPSHOT_SCROLLBACK_CAP = 5_000; +// Dedup window for self-sent keyCode-229 chars vs xterm's own onData. macOS +// WKWebView fires both our self-send and a textarea-input onData for the same +// keystroke; without coordination each keystroke doubles. +const KEY_229_DEDUP_MS = 200; +// Backstop cap on a pending queue. The 200ms window already bounds real +// growth; this just guards a runaway (e.g. a misbehaving IME flood). +const DEDUP_QUEUE_MAX = 64; +type PendingKey = { char: string; at: number }; + +function prunePending(q: PendingKey[], now: number): void { + while (q.length > 0 && now - q[0].at >= KEY_229_DEDUP_MS) q.shift(); +} + export type SlotAdapter = { resolveLeaf(leafId: number): LeafBridge | null; evictLeaf(leafId: number): void; @@ -70,15 +83,14 @@ export type Slot = { lastW: number; lastH: number; lastUsedAt: number; - // Dedup window for self-sent keyCode-229 chars vs xterm's own onData - // (see attachCustomKeyEventHandler): macOS emits both our self-send and a - // textarea-input onData for the same keystroke, so without coordination - // each keystroke doubles. The keystroke's keydown(kc=229) and textarea - // input(onData) can arrive in either order, so we dedup both directions. - lastOdChar: string | null; - lastOdAt: number; - selfSentChar: string | null; - selfSentAt: number; + // FIFOs of pending keyCode-229 keystrokes awaiting their counterpart, in + // both orderings: selfSentQueue holds chars we forwarded directly that xterm + // may still echo via onData; onDataQueue holds chars xterm already forwarded + // that a trailing keydown(kc=229) must not re-send. A single slot gets + // overwritten under fast typing (cd -> ccd), so we keep an ordered queue per + // direction and consume the oldest matching entry. + selfSentQueue: PendingKey[]; + onDataQueue: PendingKey[]; }; const slots: Slot[] = []; @@ -246,10 +258,8 @@ function createSlot(): Slot { lastW: 0, lastH: 0, lastUsedAt: 0, - lastOdChar: null, - lastOdAt: 0, - selfSentChar: null, - selfSentAt: 0, + selfSentQueue: [], + onDataQueue: [], }; term.attachCustomKeyEventHandler((event) => { @@ -272,20 +282,25 @@ function createSlot(): Slot { if (leafId229 !== null) { const bridge229 = adapter?.resolveLeaf(leafId229); if (bridge229) { + const now = Date.now(); // xterm's textarea-input onData may have already fired for this // keystroke just before keydown; if so, the char is already on its - // way and self-sending would double it. Otherwise self-send and mark - // so a trailing onData is dropped. - if ( - slot.lastOdChar === event.key && - Date.now() - slot.lastOdAt < 200 - ) { - slot.lastOdChar = null; + // way and self-sending would double it. Consume the oldest matching + // pending onData entry instead of self-sending. + prunePending(slot.onDataQueue, now); + const odIdx = slot.onDataQueue.findIndex( + (e) => e.char === event.key, + ); + if (odIdx >= 0) { + slot.onDataQueue.splice(odIdx, 1); event.preventDefault(); return false; } - slot.selfSentChar = event.key; - slot.selfSentAt = Date.now(); + // Otherwise self-send and enqueue so a trailing onData is dropped. + slot.selfSentQueue.push({ char: event.key, at: now }); + if (slot.selfSentQueue.length > DEDUP_QUEUE_MAX) { + slot.selfSentQueue.shift(); + } bridge229.writeToPty(event.key); event.preventDefault(); return false; @@ -345,21 +360,25 @@ function createSlot(): Slot { term.onData((data) => { const leafId = slot.currentLeafId; - // Drop xterm's own onData when it duplicates a char we already self-sent - // for a keyCode-229 keystroke within the dedup window. macOS fires both, - // so without this the self-send + onData would double the character. - if ( - slot.selfSentChar !== null && - data === slot.selfSentChar && - Date.now() - slot.selfSentAt < 200 - ) { - slot.selfSentChar = null; - return; + const now = Date.now(); + if (data.length === 1) { + // Drop xterm's own onData when it duplicates a char we already self-sent + // for a keyCode-229 keystroke within the dedup window. Consume the oldest + // matching pending entry so a fast burst (cd) dedups each char, not just + // the most recent one. + prunePending(slot.selfSentQueue, now); + const ssIdx = slot.selfSentQueue.findIndex((e) => e.char === data); + if (ssIdx >= 0) { + slot.selfSentQueue.splice(ssIdx, 1); + return; + } + // Record this onData so a trailing keydown(kc=229) for the same char + // knows not to self-send (onData-arrived-first ordering). + slot.onDataQueue.push({ char: data, at: now }); + if (slot.onDataQueue.length > DEDUP_QUEUE_MAX) { + slot.onDataQueue.shift(); + } } - // Record this onData so a trailing keydown(kc=229) for the same char - // knows not to self-send ( onData-arrived-first ordering ). - slot.lastOdChar = data.length === 1 ? data : null; - slot.lastOdAt = Date.now(); if (leafId === null) return; adapter?.resolveLeaf(leafId)?.writeToPty(data); }); From b10cbf9836fa65ddaa94d59340a89b7f98f0fa68 Mon Sep 17 00:00:00 2001 From: vincentzzh424 <146338078-vincentzzh424@users.noreply.github.com> Date: Wed, 1 Jul 2026 19:41:48 +0800 Subject: [PATCH 3/3] fix(pty): bound input queue, stop writes on failure, drop em dashes Address the second round of CodeRabbit review on #904: - Bound the PTY input queue with a byte cap (WRITE_QUEUE_CAP, 16 MiB) tracked via an AtomicUsize. pty_write reserves capacity before enqueueing and rolls back on send failure, so a stuck PTY (child hung, pipe full) surfaces backpressure instead of growing memory unbounded. - The writer thread now breaks on write or lock failure. Dropping the receiver closes the channel, so later pty_write calls get a channel-closed error rather than silently discarding input. - Remove em dashes from touched comments per the repo-wide convention (TERAX.md: no em-dash anywhere). Verified: cargo clippy --locked (clean), cargo test --locked (221 passed), pnpm check-types (clean), biome lint (0 new errors). Co-Authored-By: Claude --- src-tauri/src/modules/pty/mod.rs | 28 ++++++++++++---- src-tauri/src/modules/pty/session.rs | 49 ++++++++++++++++++++-------- 2 files changed, 57 insertions(+), 20 deletions(-) diff --git a/src-tauri/src/modules/pty/mod.rs b/src-tauri/src/modules/pty/mod.rs index 33b7f97d6..09726d688 100644 --- a/src-tauri/src/modules/pty/mod.rs +++ b/src-tauri/src/modules/pty/mod.rs @@ -93,9 +93,10 @@ pub async fn pty_open( // Input is the latency-critical path: raw body + id header skips JSON // serialization of every keystroke on both sides of the IPC boundary. Bytes -// are handed to the session's writer thread via an unbounded channel, so -// `send` is non-blocking and arrival order is preserved across a burst of -// keystrokes (the writer thread serializes the actual write_all calls). +// are handed to the session's writer thread via a channel, so arrival order +// is preserved across a burst of keystrokes (the writer thread serializes the +// actual write_all calls). The queue is byte-bounded: past WRITE_QUEUE_CAP we +// return a backpressure error instead of letting a stuck PTY grow memory. #[tauri::command] pub async fn pty_write( state: tauri::State<'_, PtyState>, @@ -111,6 +112,7 @@ pub async fn pty_write( return Err("pty_write: expected raw body".to_string()); }; let bytes = bytes.clone(); + let len = bytes.len(); let session = state .sessions .read() @@ -121,10 +123,22 @@ pub async fn pty_write( log::warn!("pty_write: unknown id={id}"); "no session".to_string() })?; - session.write_tx.send(bytes).map_err(|_| { - log::debug!("pty_write id={id}: writer channel closed"); - "no session".to_string() - }) + // Reserve queue capacity first so two racing writers can't both sneak past + // the cap. Rolled back on send failure (writer thread died -> channel + // closed) so a dead session reports "no session" rather than backpressure. + let prev = session.queued_bytes.fetch_add(len, Ordering::Relaxed); + if prev + len > session::WRITE_QUEUE_CAP { + session.queued_bytes.fetch_sub(len, Ordering::Relaxed); + return Err("pty_write: input backlog full".to_string()); + } + match session.write_tx.send(bytes) { + Ok(()) => Ok(()), + Err(_) => { + session.queued_bytes.fetch_sub(len, Ordering::Relaxed); + log::debug!("pty_write id={id}: writer channel closed"); + Err("no session".to_string()) + } + } } #[tauri::command] diff --git a/src-tauri/src/modules/pty/session.rs b/src-tauri/src/modules/pty/session.rs index 8f3f649cc..41ef1a5a8 100644 --- a/src-tauri/src/modules/pty/session.rs +++ b/src-tauri/src/modules/pty/session.rs @@ -1,5 +1,5 @@ use std::io::{Read, Write}; -use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use std::sync::mpsc::{self, Sender}; use std::sync::{Arc, Condvar, Mutex}; use std::thread; @@ -30,21 +30,26 @@ const MAX_PENDING: usize = 4 * 1024 * 1024; // we're forced to discard backlog. const OVERFLOW_NOTICE: &[u8] = b"\x1bc\x1b[2m[terax: dropped output due to backpressure]\x1b[0m\r\n"; +// Cap on bytes queued for the PTY input writer. A stuck PTY (child hung, pipe +// full) would otherwise let pty_write accumulate unbounded Vecs in memory; +// once exceeded, pty_write returns a backpressure error instead of queuing. +// 16 MiB is far above any realistic keystroke burst or paste. +pub(super) const WRITE_QUEUE_CAP: usize = 16 * 1024 * 1024; pub struct Session { // Field drop order is intentional. Rust drops fields top-to-bottom: - // 1. `_job` — on Windows, closing the Job HANDLE fires + // 1. `_job`: on Windows, closing the Job HANDLE fires // KILL_ON_JOB_CLOSE, terminating the pwsh tree before the master // pipe drops. Without this, ClosePseudoConsole in `master`'s Drop // can block waiting for conhost to drain pending output, freezing // the Tauri worker thread that triggered the close. - // 2. `killer` — best-effort kill (redundant on Windows once Job + // 2. `killer`: best-effort kill (redundant on Windows once Job // closed, but harmless and required on Unix where there is no Job). - // 3. `write_tx` — drops the last channel sender, so the dedicated + // 3. `write_tx`: drops the last channel sender, so the dedicated // writer thread drains and exits, releasing the input-side writer // clone it holds. (The reader thread's writer clone, used for DA // replies, is released when the killed child's EOF unwinds it.) - // 4. `master` — last; ClosePseudoConsole on Windows. By now the child + // 4. `master`: last; ClosePseudoConsole on Windows. By now the child // is dead and conhost has nothing left to drain. #[cfg(windows)] _job: Option, @@ -55,6 +60,10 @@ pub struct Session { // thread, so a burst of pty_write calls stays in arrival order rather than // racing for the writer lock from independent spawn_blocking jobs. pub write_tx: Sender>, + // Bytes currently queued in write_tx but not yet written. Bounded by + // WRITE_QUEUE_CAP: pty_write refuses to enqueue past the cap so a stuck + // PTY can't grow memory unbounded. + pub queued_bytes: Arc, pub master: Mutex>, // Set by the waiter once the child exits, so pty_open can reap a shell // that died before it was registered. @@ -144,20 +153,31 @@ pub fn spawn( )); guard.disarm(); - // Dedicated writer thread: serializes all input writes in arrival order - // so concurrent pty_write calls can't reach `writer.lock()` out of order - // and scramble a fast burst of keystrokes. Unbounded channel => send never - // blocks the IPC caller; the thread exits when the last sender drops. + // Dedicated writer thread: serializes all input writes in arrival order so + // concurrent pty_write calls can't reach the writer lock out of order and + // scramble a fast burst of keystrokes. The channel is byte-bounded (see + // queued_bytes / WRITE_QUEUE_CAP); pty_write applies backpressure instead + // of letting a stuck PTY accumulate unbounded input. The thread exits when + // the last sender drops (session close) or a write/lock fails: breaking + // drops the receiver so later sends return a channel-closed error rather + // than silently discarding input. let (write_tx, write_rx) = mpsc::channel::>(); let writer_for_writer = writer.clone(); + let queued_bytes = Arc::new(AtomicUsize::new(0)); + let queued_bytes_writer = queued_bytes.clone(); thread::Builder::new() .name("terax-pty-writer".into()) .spawn(move || { for bytes in write_rx { - if let Ok(mut w) = writer_for_writer.lock() { - if let Err(e) = w.write_all(&bytes) { - log::debug!("pty writer thread write failed: {e}"); - } + let len = bytes.len(); + let ok = match writer_for_writer.lock() { + Ok(mut w) => w.write_all(&bytes).is_ok(), + Err(_) => false, + }; + queued_bytes_writer.fetch_sub(len, Ordering::Relaxed); + if !ok { + log::debug!("pty writer thread stopping after write/lock failure"); + break; } } }) @@ -185,6 +205,7 @@ pub fn spawn( shell_pid, killer: Mutex::new(killer), write_tx, + queued_bytes, master: Mutex::new(pair.master), exited: exited.clone(), }); @@ -371,6 +392,7 @@ mod tests { shell_pid: child.process_id().unwrap_or(0), killer: Mutex::new(killer), write_tx: mpsc::channel().0, + queued_bytes: Arc::new(AtomicUsize::new(0)), master: Mutex::new(pair.master), exited: Arc::new(AtomicBool::new(false)), }); @@ -419,6 +441,7 @@ mod tests { shell_pid: 0, killer: Mutex::new(killer), write_tx: mpsc::channel().0, + queued_bytes: Arc::new(AtomicUsize::new(0)), master: Mutex::new(pair.master), exited: Arc::new(AtomicBool::new(false)), });