Repository navigation
fix(desktop): move settings-store writes off the main thread - #237
Merged
Merged
Conversation
conf v15's store.set is a synchronous fsync on the Electron main thread, and one of those parked the whole app in uninterruptible disk sleep when a kernel journal stall lost its wakeup (an NVMe write the drive never completed wedged ext4's journal; fast-commit then dropped the waiter): event loop dead, IPC dead, quit impossible. Renderers persist through store-set at exactly the busy moments, e.g. a sandbox mount-apply restart, which is where it was hit live — twice. getStore now returns a small fork-owned file store: state lives in memory (read-your-writes; conf's reread-per-access design would let a set on one key load a stale file and drop a still-queued update, which is why the stock class could not simply be rerouted), and every change flushes write-behind through a serialized latest-wins async atomic writer (temp file -> fsync -> rename via fs.promises, so a wedged disk stalls a threadpool thread while the app stays interactive). The on-disk format is unchanged and existing files load as-is; a corrupt file starts empty with a logged error instead of throwing from every accessor. stopSidecars awaits a bounded flushStoreWrites(1500) so relaunch/exit keep pending settings without letting a wedged disk hang shutdown; deleting a store file discards its writer first so a late flush cannot resurrect it. The electron-store dependency is removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Problem
Twice on a dev machine, clicking Save in the sandbox Folders dialog froze the entire app beyond quitting. Kernel forensics (
sysrq-w) showed the Electron main thread in uninterruptible disk sleep insidefsync(ext4_fc_commit → jbd2_log_wait_commit): the NVMe dropped a write during a power-state transition, wedging the ext4 journal, and a fast-commit bug in the 6.8 kernel then lost the wakeup — so the main thread never returned even after the journal recovered. With the event loop dead, timers, IPC, and quit were all gone.The syscall that walks into the trap is ours to control: conf v15 (electron-store's engine) performs a synchronous fsync on the main thread for every
store.set, and renderers persist throughstore-setIPC at exactly the busy moments — e.g. when a sandbox connection drops during a mount-apply restart.Change
getStore()now returns a small fork-owned store instead of electron-store:store-file.ts— state lives in memory (read-your-writes); every mutation flushes write-behind. Replacing rather than patching is load-bearing: conf rereads the file on every access, so with queued writes aseton one key would load a stale file and silently drop a still-queued update to another (covered by a test).store-writer.ts— serialized, latest-wins async atomic writer (temp file → fsync → rename viafs.promises). The fsync runs on the libuv threadpool: a wedged disk stalls one pool thread while the app stays interactive, and the flush lands when the disk recovers.stopSidecarsawaitsflushStoreWrites(1500)— relaunch/SIGINT/SIGTERM keep pending settings, bounded so a wedged disk can't hang quit. Deleting a store file discards its writer first so a late flush can't resurrect it.electron-storedependency removed.Trade-off accepted: a hard crash can lose the last few milliseconds of unflushed settings — versus today's behavior of freezing the entire app permanently on a bad disk.
Tests
bun typecheck+bun testinpackages/desktop: 96 pass (13 new — writer coalescing/timeout/discard/error paths; store read-your-writes, the cross-key lost-update scenario, format compatibility both directions, corrupt/missing file, delete/clear).bun run build(electron-vite) passes without electron-store.🤖 Generated with Claude Code