diff --git a/memory.md b/memory.md index 94ec740..97c730c 100644 --- a/memory.md +++ b/memory.md @@ -127,6 +127,13 @@ Smoke test on Windows revealed bugs specific to detached (non-main) windows. Mai - **Native scrollbars (fixed).** `::-webkit-scrollbar` theming was scoped to `.xterm-viewport` only; made it global (`*::-webkit-scrollbar` + `* { scrollbar-width/color }`) so the tab strip / panels / menus match the dark theme. - **Capability fix recap:** `default.json` `"windows": ["main", "pane-window-*"]` — the load-bearing fix for the whole detached-window feature (B2–B5). Confirmed: app-defined Tauri commands aren't individually permission-gated; they're available to any window the capability is *applied* to (listed in `windows`). +**Pre-release audit (2026-05-28, 3-agent fan-out) — findings + dispositions:** + +- **(FIXED, medium) XtermPane IPC listener leak on unmount-during-await.** After `unlistenData = await onPaneData(...)` / `unlistenExit = await onPaneExit(...)` there was no `destroyed` re-check, so if the pane unmounted during the await (StrictMode, fast moveToNewWindow/closeTab) the sync cleanup had already captured a null unlisten and the `pane://{id}/data`/exit subscription leaked. Added `if (destroyed) { unlistenData?.(); unlistenExit?.(); return; }` after each assignment in both adopt and spawn paths. +- **(DEFERRED, high — known low-risk) Transferred-PTY/refcount leak if a detached window closes mid-adopt.** `mark_pane_transferring` bumps a refcount that suppresses `kill`; only `claim_pane` (from the target XtermPane mount) drops it. The `CloseRequested` handler (lib.rs:74) forgets workspaces + pending-init but never releases the refcount or kills the pane → if the window closes before adopt's `claim_pane`, that PTY + reader thread leak for the app lifetime. **In practice very low-probability**: adopt of a transferred pane is near-instant (paneId known synchronously, no spawn wait), so `claim` runs within ms of mount — by the time a user sees and closes the window, it's already claimed. User chose ship-now. **Proper fix when revisited:** keep a `label→paneId` "adopting" registry (set when `take_pending_window_init` consumes the payload, cleared by `claim_pane`), and have the close handler force-kill (drop refcount + kill) any still-unclaimed paneId for the closing label. The unconsumed-pending-init subset can be handled more cheaply (close handler already has the PendingInit.pane_id when the entry is still present). +- **(NOT FIXING, low) waitForPaneRegistration doesn't settle on early unmount** — `registerPaneId(leafId, null)` doesn't reject a pending waiter, so moveToNewWindow/MCP-spawn stalls until the timeout instead of failing fast. Functionally safe (timeout fires). +- tabs/LeafPane/TabStrip reviewer: no findings. + 2. `pnpm tauri dev` — smoke test: - Existing workspace loads as one tab named "Default" ✓ migrate - Ctrl+T spawns new tab with default-shell pane diff --git a/src/components/XtermPane.tsx b/src/components/XtermPane.tsx index 3b90015..de4309a 100644 --- a/src/components/XtermPane.tsx +++ b/src/components/XtermPane.tsx @@ -186,11 +186,22 @@ export default function XtermPane({ term?.write(b64ToBytes(b64)); onDataReceivedRef.current?.(); }); - if (destroyed) return; + // `destroyed` may have flipped during the await — the sync cleanup + // already ran and captured a null unlisten, so unlisten here or the + // subscription leaks. + if (destroyed) { + unlistenData?.(); + return; + } unlistenExit = await onPaneExit(paneId, () => { term?.write("\r\n\x1b[33m[pane exited]\x1b[0m\r\n"); onStatusRef.current?.(`pane ${paneId} exited`, false); }); + if (destroyed) { + unlistenData?.(); + unlistenExit?.(); + return; + } // Match the PTY to our cell grid (the source window may have had // different dimensions). try { @@ -227,11 +238,20 @@ export default function XtermPane({ term?.write(b64ToBytes(b64)); onDataReceivedRef.current?.(); }); + if (destroyed) { + unlistenData?.(); + return; + } unlistenExit = await onPaneExit(paneId, () => { term?.write("\r\n\x1b[33m[pane exited]\x1b[0m\r\n"); onStatusRef.current?.(`pane ${paneId} exited`, false); }); + if (destroyed) { + unlistenData?.(); + unlistenExit?.(); + return; + } } term?.onData((data) => {