Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

using signal-exit in Electron causes Ctrl+C in terminal to stop working #49

Closed
astoilkov opened this issue Jan 22, 2018 · 4 comments
Closed

Comments

@astoilkov
Copy link

astoilkov commented Jan 22, 2018

Running the code below in Electron app causes hitting Ctrl+C in the terminal to not close Electron.

window.onbeforeunload = function () {};

let onExit = require('signal-exit');
let remove = onExit(() => 1);

// tried to call these to see if the issue will be fixed but the lines below don't affect the problem
remove();
onExit.unload();

I tried to isolate the problem to found out if it's a problem of Electron or problem of signal-exit but I wasn't able to. I tried subscribing to process events and overriding process.emit and process.reallyExit but without success.

@tcodes0
Copy link

tcodes0 commented Jul 21, 2018

Please have a look

This comment and the thread

This PR

@astoilkov
Copy link
Author

From what I can understand the problem is caused by the hideCursor module. However, I am not sure what is causing the problem in Electron because it doesn't use the hideCursor module by default.

starpit added a commit to starpit/madwizard that referenced this issue Feb 1, 2023
when `ora` hides the cursor... it installs a SIGINT handler to clean up the hiding of the cursor. this causes major unreliability with ctrl+c handling.

```
  // re: hideCursor: false, ugh, otherwise ctrl+c doesn't work reliably
  // sindresorhus/ora#27
  // tapjs/signal-exit#49
```
starpit added a commit to guidebooks/madwizard that referenced this issue Feb 2, 2023
when `ora` hides the cursor... it installs a SIGINT handler to clean up the hiding of the cursor. this causes major unreliability with ctrl+c handling.

```
  // re: hideCursor: false, ugh, otherwise ctrl+c doesn't work reliably
  // sindresorhus/ora#27
  // tapjs/signal-exit#49
```
@jakzo
Copy link

jakzo commented Mar 8, 2023

I'm running into this issue too. In my case I was trying to use Ink with ts-node or tsx (both wouldn't quit on CTRL-C). It seems that the problem is caused by a deadlock between signal-exit and another module (eg. tsx) both listening for SIGINT then continuing to exit the process but only if there are no other listeners.

My thought process when thinking about a solution for this is that ideally node would expose an API to listen for SIGINT events without changing the default behaviour of exiting the program but without that signal-exit, tsx and every other tool with this use-case needs to put themselves in charge of deciding whether to continue exiting or not. This will only ever work if one program is in charge so they would somehow need to decide this amongst themselves.

One way to achieve this would be to make a small process-on-exit module which does the decision logic on SIGINT/SIGTERM and getting all these tools (signal-exit, tsx, etc.) to use that instead of handling it themselves, but that is pretty close to what signal-exit already does, so I'm thinking it would be better to get other tools to use signal-exit instead.

So in summary I think the best solution is for all tools which listen for SIGINT/SIGTERM to be updated to use signal-exit for this instead of handling it themselves.

@isaacs
Copy link
Member

isaacs commented Apr 15, 2023

Hah, yeah, it looks like we're all trying to stay out of each others' way, so no one goes first, like a bunch of overly polite people who can't walk through a door.

I agree that the best would be for anyone trying to implement that behavior to use this module, which just does that one thing, but I'm not sure how to make it work otherwise. Maybe a "ignoreOtherSignalHandlers" option, or something? Without some explicit indication, there's no way to know whether a handler was intending to be unobtrusive or actually handle the event.

One idea would be for signal-exit to be even more sneaky in how it assigns listeners, by hackily monkey-patching process.emit (which signal-exit already does for the 'exit' even lol), but the process EventEmitter interface has some special behavior for signals, so there's no way to have a listener that isn't listed in the process.listeners() list (when the number of signal listeners drops to zero, Node doesn't respond to that signal at all).

So really no other way forward with this, I think.

@isaacs isaacs closed this as completed Apr 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants