fix(dev): forward termination signals to Electron child (#899) - #900
Open
iddogino wants to merge 1 commit into
Open
fix(dev): forward termination signals to Electron child (#899)#900iddogino wants to merge 1 commit into
iddogino wants to merge 1 commit into
Conversation
Spawn the Electron binary directly in dev/preview mode but forward SIGINT/SIGTERM/SIGHUP from the electron-vite parent to the child, matching Electron's own CLI wrapper (node_modules/electron/cli.js). Without this, Ctrl-C sends SIGINT to the foreground process group once: the Node parent exits immediately (no handler) and the Electron child gets only a single SIGINT. On macOS, an Electron app whose before-quit handler calls preventDefault() and later calls app.quit() can stall in AppKit's event loop in that path. Under a direct 'electron .' launch the CLI shim forwards SIGINT as well, so Electron gets it twice and exits cleanly. Matching that behavior here fixes the hang. Fixes alex8088#899
Owner
|
Thanks for the PR. I will perform some local tests to verify the changes. |
|
Have you had a chance to review this @alex8088 ? I have a use-case that will benefit from it as my main launches a few subprocesses and doesn't know to kill them without this patch. Cheers! |
7 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
Fixes #899. In
startElectron, forwardSIGINT/SIGTERM/SIGHUPfrom the electron-vite parent to the spawned Electron child, matching Electron's own CLI wrapper (node_modules/electron/cli.js).Why
electron-vite spawns the Electron binary directly via
spawn(electronPath, ...)and does not trap signals. When the user hits Ctrl-C in a dev session:Under a direct
./node_modules/.bin/electron .launch the picture is different: the Electron CLI shim installs a handler that forwards SIGINT to its child, so the Electron child receives SIGINT twice (once from the pgroup, once forwarded). On macOS, an Electron app whosebefore-quithandler callsevent.preventDefault()and then later callsapp.quit()from an async continuation can stall in AppKit's event loop — never reaching[NSApp terminate:]. The second SIGINT forwarded by the shim is what knocks Electron out of that stuck state.By matching that forwarding behavior in
electron-vite,electron-vite devnow exits as cleanly aselectron .does for that app.A reduced test was run against the repro in the issue (https://github.com/iddogino/electron-vite-quit-repro):
Notes on the implementation
currentElectronPstracks the latest-spawned child so that when the watch-on-rebuild path (server.ts) kills the old Electron and spawns a new one, signals go to the current child, not a stale reference.signalHandlersInstalledguards against accumulatingprocess.onhandlers across dev restarts.ps.on('close', process.exit)behavior is unchanged, so a normal in-app quit still exits the parent as before.Test plan
pnpm run lint,pnpm run typecheck,pnpm run buildall pass.prettier --check src/electron.tspasses.distdropped into the repro — pty Ctrl-C exits cleanly (0 lingering Electron processes).src/main/index.js, confirm restart still works).