Optimizations to prevent electron crashing#7488
Merged
Conversation
Contributor
Author
|
Putting this in draft while I continue testing |
2 tasks
351fc95 to
460e324
Compare
Contributor
Author
|
accidentally merged, reverted in main reopening |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verified these changes do help, stress tested the app for several days without a crash. I was able to repro the crash before these changes consistently with the same stress test.
fixes #6456
Electron desktop app was crashing with EXC_BREAKPOINT (SIGTRAP) during long
running sessions. Two separate memory problems were found across the main
process and renderer process.
RENDERER PROCESS — unbounded session caches:
The resultsCache Map in useChatStream.ts stored every visited session's full
message history forever with no eviction. Added a weight-based LRU cache
capped at 5 sessions that uses cheap message counting (tool messages weighted
3x) to skip caching oversized conversations, and only writes to cache when
chat state is idle to avoid running on every streamed chunk. Wired the
existing activeSessions tab eviction in App.tsx to also clear evicted
sessions from the cache. Deleted sessionCache.ts which was dead code never
imported anywhere.
MAIN PROCESS — goosed stderr flooding V8 heap (the actual crash):
The goosed child process emits all tracing output to stderr. Over a 3-hour
session this produced 2GB of output. The main process was calling
Buffer.toString() on every stderr chunk, splitting into lines, pushing every
line into an unbounded errorLog array, and logging every stdout chunk
unconditionally. This filled the V8 heap until string allocation failed.
Fixed by: only converting stderr buffers to strings during the first 1MB of
startup (needed for fatal error detection) or when the raw bytes contain a
fatal error pattern checked via Buffer.includes — avoiding string conversion
entirely for 99.9% of stderr. Capped errorLog at 200 lines. Added a size
guard on stdout logging while preserving the cert fingerprint extraction
that main added for TLS support.
MAIN PROCESS — other stream handlers (from Rabi's PR #7509):
The read-file IPC handler was spawning cat and calling data.toString() in
stream data callbacks on non-Windows platforms — replaced with fs.readFile
on all platforms which does a single Buffer read with no per-chunk string
allocation. The check-ollama handler was doing string concatenation in data
callbacks — changed to accumulate chunks and join on stream close.