Fix shutdown crash (EXC_BREAKPOINT/SIGTRAP) and reduce startup/RAM footprint - #630
Open
temperatio wants to merge 11 commits into
Open
Fix shutdown crash (EXC_BREAKPOINT/SIGTRAP) and reduce startup/RAM footprint#630temperatio wants to merge 11 commits into
temperatio wants to merge 11 commits into
Conversation
_extensionManifestCache and _extensionBundleCodeCache were plain Maps with no size limit, so a long session opening many distinct extensions accumulated their full bundled text in memory indefinitely. Cap both at a bounded LRU (200 manifests, 40 bundles).
callWindowManagerWorker() already forks the worker lazily on first use via ensureWindowManagerWorker(). Most sessions never touch window management, so eagerly forking a full Node child process at app.whenReady() just to save latency on a feature most users never hit isn't worth the constant RAM cost.
The parakeet/qwen3/whisper.cpp persistent serve-mode processes keep a model loaded in memory for fast repeat transcription, but previously stayed alive until the app quit once started even once. Add a shared idle-shutdown scheduler that kills each one after 10 minutes without use, reset on every real invocation.
discoverInstalledExtensionCommands() now yields to the event loop every 5 extensions instead of scanning the whole install in one uninterrupted synchronous burst, and initCommandsCache()/loadCommandsDiskCache() move to fs.promises with icon re-attachment done concurrently via Promise.all instead of one fs.readFileSync per command — both run right before createWindow() on every launch.
node-window-manager requires both at runtime to load its native addon and resolve file icons, but only node-window-manager itself was listed in asarUnpack. In any packaged build the require() call crossed from the unpacked filesystem location back into the still-packed asar and failed, so window-manager-worker responded "window manager unavailable" to every request — breaking window management, "restore previous app" capture, and window layout commands in every distributable build. Found by actually running a packaged .dmg and reproducing the failure with ELECTRON_RUN_AS_NODE=1, not from code review.
Extension bundling (esbuild + fs) and script command execution (child_process) run real Node async work on the main process. If one of those callbacks completed after Electron began tearing down the V8/Node environment during quit, Node aborted natively (EXC_BREAKPOINT/SIGTRAP) instead of throwing a catchable JS error — reproduced repeatedly in production crash reports (SuperCmd-2026-07-13-110438.ips and others, all identical signature: CrBrowserMain thread, CrShutdownDetector present, no ASI message). Track every getExtensionBundle/executeScriptCommand/buildAllCommands call in a registry. before-quit now intercepts the first quit attempt, waits up to 2s for tracked work to settle, and only then lets the app actually quit — bounded so a stuck extension can't block shutdown indefinitely. See the ADR stored for this project (Users-cgomez-orca-workspaces- SuperCmd-Fix-shoutdown) via codebase-memory for the full crash diagnosis and the alternatives considered.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c4b808f405
ℹ️ 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".
ensureXServer() (parakeet/qwen3/whisper.cpp) bumped the 10-minute idle timer once at the start, before the request was even sent, and nothing re-armed it while a request was in flight. A transcription running past 10 minutes (long audio, slow hardware) got its server killed and its pending request rejected mid-flight, even though the server was busy, not idle. createIdleShutdownScheduler now takes an isBusy() predicate; when the timer fires while a request is still pending it defers instead of killing, and only shuts the server down once it's genuinely idle.
lastBuildError only ever grew via .set() on build failure and was never evicted on a later successful rebuild of the same command, so it retained stale error strings for the lifetime of the process.
Gated behind SC_HEAP_DEBUG=1 so it's inert for regular users. Logs process.memoryUsage() every 5 minutes and registers Cmd+Alt+Shift+H to write a heap snapshot, to help confirm whether the main-process heap grows unbounded during a long session (investigating the 2026-07-17 EXC_BREAKPOINT crash on CrBrowserMain).
The window-manager worker was forked with stdio fully ignored, so any underlying error (e.g. node-window-manager failing to load) was silently dropped — callers only ever saw the generic "window manager unavailable" message. Pipe stdout/stderr back into the main process console instead.
Reproduced the original crash signature live: after a long session the
main-process heap climbed toward V8's default old-space ceiling
(~2GB), Mark-Compact GC could no longer reclaim enough, and the process
aborted with SIGTRAP — the same signal from the original crash report.
Raising --max-old-space-size via app.commandLine gives it headroom on
machines that have it to spare, buying time while the underlying
retainer is tracked down separately.
Also switch the SC_HEAP_DEBUG snapshot capture to a SIGUSR2 handler (in
addition to the existing hotkey) so a snapshot can be requested from a
script/terminal without the app needing focus, and write snapshots to
os.tmpdir() instead of app.getPath('logs') — the logs dir has been
observed losing files to unrelated cleanup shortly after capture.
3 tasks
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
Six independent, atomic commits (each safe to review/revert on its own):
perf(extensions): cap manifest/bundle caches with LRU eviction—_extensionManifestCache/_extensionBundleCodeCachewere unboundedMaps; a long session opening many distinct extensions accumulated their full bundled text in memory indefinitely. Capped at 200/40 entries via a small LRU.perf(main): stop pre-warming window-manager-worker at startup— the worker was force-forked atapp.whenReady()"just in case";callWindowManagerWorker()already forks it lazily on first real use, so the eager fork was pure idle RAM for sessions that never touch window management.perf(main): make extension discovery and commands cache non-blocking—discoverInstalledExtensionCommands()now yields to the event loop every 5 extensions instead of scanning the whole install in one synchronous burst;initCommandsCache()/loadCommandsDiskCache()move tofs.promiseswith icon re-attachment done concurrently viaPromise.allinstead of onereadFileSyncper command, right beforecreateWindow().perf(main): auto-shutdown idle transcription servers after 10 minutes— the parakeet/qwen3/whisper.cpp persistent serve processes kept a model loaded in memory indefinitely once started once. They now self-terminate after 10 minutes of no use.fix(build): unpack extract-file-icon and node-gyp-build from asar— found while testing feat: support openai compatible APIs #2 above:node-window-managerrequires both at runtime for its native addon, but onlynode-window-manageritself was inasarUnpack. In any packaged build this broke all window management (list/get windows, layout commands, "restore previous app" capture) with a silentwindow manager unavailableresponse. Reproduced withELECTRON_RUN_AS_NODE=1requiring the module directly from the packaged app — not a code-review guess.fix(main): drain in-flight extension/script work before quitting— the actual crash fix. Multiple production crash reports (e.g.SuperCmd-2026-07-13-110438.ips) showEXC_BREAKPOINT/SIGTRAPon theCrBrowserMainthread with no application-specific message, consistent with a Node callback (fs/child_process from extension bundling or script execution) completing after Electron begins tearing down the V8/Node environment on quit.before-quitnow tracks in-flightgetExtensionBundle/executeScriptCommand/buildAllCommandscalls and waits up to 2s for them to settle before actually quitting, bounded so a stuck extension can't block shutdown indefinitely.Why bundled into one PR
Per CONTRIBUTING.md a PR should normally address one concern — happy to split into 6 separate PRs if preferred, each commit is self-contained and was verified independently. Bundled here at the requester's preference since all six came out of the same investigation session.
Compatibility impact
No public API surface changes. No changes to
@raycast/api/@raycast/utilsshims. Commit 6 adds a bounded (max 2s) delay to app quit only when extension/script work is genuinely in flight at that moment; normal quits (nothing pending) are unaffected.How this was tested
npm run build/tsc --noEmit -p tsconfig.main.jsonclean after every commit..dmgpackages (npm run package:unsigned) and separate dev-mode instances (electron .), driving them for real rather than unit-testing the exported functions:ELECTRON_RUN_AS_NODE=1requiringnode-window-managerdirectly from the packaged, unpacked asar path, then confirming it resolves cleanly after the fix.ps aux) — nowindow-manager-worker.jschild present until first real use.[Commands] Loaded N commands from disk cache,Discovered N apps... in Nms), no regressions.~/Library/Logs/DiagnosticReports.before-quitfires) from outside the app — this requires either exact timing luck or in-app UI automation not available in this environment. The mechanism itself (Promise.racewith a bounded timeout, guarded against re-entrant infinite loops) was verified by code review and by confirming it does not regress the common no-op path.Test plan for reviewers
~/Library/Logs/DiagnosticReports..dmgbuild, not justnpm run dev.ps aux | grep -E "parakeet |qwen3|whisper").