fix(terminal): prevent crash during resize with high-output programs#114
fix(terminal): prevent crash during resize with high-output programs#1140xBigBoss wants to merge 2 commits intocoder:mainfrom
Conversation
Fixes a race condition that causes crashes when resizing the terminal while programs with high output (like cmatrix, htop) are running. The issue occurs because the render loop and write operations can access WASM memory buffers while resize() is reallocating them, causing SIGSEGV crashes. This fix implements three protections: 1. **Pause render loop during resize**: Cancel the animation frame before WASM resize and restart after, preventing concurrent buffer access. 2. **Invalidate cached buffer views**: Clear graphemeBuffer and graphemeBufferPtr when invalidating buffers, since TypedArray views become detached when underlying memory is reallocated. 3. **Queue writes during resize**: Buffer incoming PTY data during resize and flush after a frame, preventing writes from hitting WASM while buffers are being reallocated. All three protections are needed to fully prevent the race condition with rapidly-outputting programs.
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf5df363bd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| this.resizeFlushFrameId = requestAnimationFrame(() => { | ||
| this.resizeFlushFrameId = undefined; | ||
| this.isResizing = false; | ||
| this.flushWriteQueue(); |
There was a problem hiding this comment.
isResizing is only cleared inside a requestAnimationFrame callback, so after any resize in a background/hidden tab (where rAF is paused or heavily throttled) every subsequent write() stays queued and never reaches WASM until the tab becomes visible again. In high-output sessions this causes apparent terminal hangs and unbounded writeQueue growth, which is a production-impacting regression for embedded terminals that keep running while not foregrounded.
Useful? React with 👍 / 👎.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Cancel the render loop before WASM resize to prevent accessing detached TypedArray views when buffers are reallocated. Restart render loop and flush any queued writes synchronously after resize completes. This avoids the background-tab regression from PR #114's approach of using an isResizing flag cleared via requestAnimationFrame (rAF is throttled/paused in background tabs, causing writes to queue indefinitely). Changes: - terminal.ts: Add cancelRenderLoop()/flushWriteQueue(), wrap resize() in try/catch with render loop pause/restart, add defensive writeQueue - ghostty.ts: Invalidate grapheme buffers in invalidateBuffers() alongside viewport buffers to prevent stale TypedArray access Co-authored-by: 0xBigBoss <95193764+0xBigBoss@users.noreply.github.com>
Cancel the render loop before WASM resize to prevent accessing detached TypedArray views when buffers are reallocated. Restart render loop and flush any queued writes synchronously after resize completes. This avoids the background-tab regression from PR #114's approach of using an isResizing flag cleared via requestAnimationFrame (rAF is throttled/paused in background tabs, causing writes to queue indefinitely). Changes: - terminal.ts: Add cancelRenderLoop()/flushWriteQueue(), wrap resize() in try/catch with render loop pause/restart, add defensive writeQueue - ghostty.ts: Invalidate grapheme buffers in invalidateBuffers() alongside viewport buffers to prevent stale TypedArray access Co-authored-by: 0xBigBoss <95193764+0xBigBoss@users.noreply.github.com> * Guard startRenderLoop() against duplicate animation frame loops Add early return if animationFrameId is already set, preventing multiple concurrent RAF loops when resize() is called re-entrantly from an onResize listener. --------- Co-authored-by: 0xBigBoss <95193764+0xBigBoss@users.noreply.github.com>
|
@0xBigBoss thanks for the contributions! Sorry it took so long to merge, I addressed the remaining changes in #132 and dropped you a co-author |
3 participants
Summary
Fixes a race condition that causes crashes when resizing the terminal while programs with high output (like cmatrix, htop) are running.
The issue occurs because the render loop and write operations can access WASM memory buffers while
resize()is reallocating them, causing SIGSEGV crashes.Changes
This PR implements three protections:
Pause render loop during resize: Cancel the animation frame before WASM resize and restart after, preventing concurrent buffer access.
Invalidate cached buffer views: Clear
graphemeBufferandgraphemeBufferPtrwhen invalidating buffers, since TypedArray views become detached when underlying memory is reallocated.Queue writes during resize: Buffer incoming PTY data during resize and flush after a frame, preventing writes from hitting WASM while buffers are being reallocated.
All three protections are needed to fully prevent the race condition with rapidly-outputting programs.
Testing
cmatrix- no longer crashesFiles Changed
lib/terminal.ts- Main resize protection logiclib/ghostty.ts- Buffer invalidation for graphemeBuffer