Skip to content
This repository has been archived by the owner on Apr 13, 2024. It is now read-only.

SIGINT (Ctrl-C) always quits shell while running in Node #65

Closed
AtkinsSJ opened this issue Mar 19, 2024 · 4 comments · Fixed by #71
Closed

SIGINT (Ctrl-C) always quits shell while running in Node #65

AtkinsSJ opened this issue Mar 19, 2024 · 4 comments · Fixed by #71

Comments

@AtkinsSJ
Copy link
Contributor

This code, which we use to intercept Ctrl-C, unconditionally terminates Phoenix:

if (input.length === 1 && (input[0] === signals.SIGINT || input[0] === signals.SIGQUIT)) {
process.exit(1);
return;
}

This means we never allow Phoenix to handle that itself, either to terminate a running foreground process, or cancel the user's current command input.

I couldn't figure out how to make this work correctly. I think we should remove that code (or maybe limit it to the kill signals - side note, I get the impression our SIG values are wrong?) and then handle the signals inside readline() somehow, but I'm not sure. @KernelDeimos Any thoughts?

@KernelDeimos
Copy link
Collaborator

We need to make sure it's possible to exit the shell. Any bugs related to not being able to exit the shell are very frustrating. I'd recommend this:

  • temporarily for development: create an async poll interval; as soon as the file /some/path/exit-shell exists, then process.exit(1)
  • remove this and see what happens when you type Ctrl+C
  • add SIGINT handling to readline so we can exit the shell when no child process is running

@KernelDeimos
Copy link
Collaborator

side note, I get the impression our SIG values are wrong?

signals.js has SIGINT: 2 and SIGQUIT: 3. I believe these are correct, but none of the other signals (like SIGWINCH) are in there yet.

@AtkinsSJ
Copy link
Contributor Author

side note, I get the impression our SIG values are wrong?

signals.js has SIGINT: 2 and SIGQUIT: 3. I believe these are correct, but none of the other signals (like SIGWINCH) are in there yet.

Those do seem correct actually... I'm not sure what made me think they weren't, but something was behaving oddly.

@AtkinsSJ
Copy link
Contributor Author

OK, I think my issue was that Ctrl-C makes Node give us the input 3, which is the ETX character, not SIGINT. But we interpreted it as a signal:

if (input.length === 1 && (input[0] === signals.SIGINT || input[0] === signals.SIGQUIT)) {
process.exit(1);
return;
}

It just happens that SIGQUIT is also 3, so the code seemed to work. I just misunderstood what Node was passing us there.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants