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

SIGINT is not handled when a short-lived process is run in watch-mode #51466

Open
azrsh opened this issue Jan 14, 2024 · 6 comments
Open

SIGINT is not handled when a short-lived process is run in watch-mode #51466

azrsh opened this issue Jan 14, 2024 · 6 comments
Labels
watch-mode Issues and PRs related to watch mode

Comments

@azrsh
Copy link

azrsh commented Jan 14, 2024

Version

v20.11.0

Platform

Linux hostname 6.5.7-arch1-1 #\1 SMP PREEMPT_DYNAMIC Tue, 10 Oct 2023 21:10:21 +0000 x86_64 GNU/Linux

Subsystem

No response

What steps will reproduce the bug?

  1. Write some short-lived script and save it to index.js.
console.log("short-lived!");
  1. Run this script in watch-mode.
$ node --watch index.js
(node:1191877) ExperimentalWarning: Watch mode is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
short-lived!
Completed running 'index.js'

After following the above steps, you will no longer be able to terminate the Node.js process in watch-mode with SIGINT or SIGTERM.

How often does it reproduce? Is there a required condition?

If you follow the steps, it will always reproduce.

What is the expected behavior? Why is that the expected behavior?

A Node.js process in watch-mode will be terminated if we send SIGINT or SIGTERM to the process, such as with Ctrl + C.

What do you see instead?

$ vim index.js
$ cat index.js
console.log("short-lived!");
$ node test.js
short-lived!
$ node --watch index.js
(node:1191877) ExperimentalWarning: Watch mode is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
short-lived!
Completed running 'index.js'
^C^C^C^C^C^C

I input Ctrl + C but the input is ignored.

Additional information

Mechanism of this behavior

The cause of this behavior is the killAndWait function located in /lib/internal/main/watch_mode.js:69.

Node.js process in watch-mode triggers a signal handler when it receives SIGINT or SIGTERM. In this signal handler, the killAndWait function is called as shown below.

const exitCode = await killAndWait(signal, true);

At this time, since the argument force of the killAndWait function is set to true, processing continues without satisfying the condition in the code path below of the killAndWait function.

if ((child.killed || exited) && !force) {
return;
}

As a result, the killAndWait function will wait for a process that has already terminated to terminate. In other words, the wait never ends.

const onExit = once(child, 'exit');
child.kill(signal);
const { 0: exitCode } = await onExit;

My proposed modifications

Modify the killAndWait function so that if exited is true, return the killAndWait function regardless of the value of the argument force.
If you accept this modification, I can create a PR!

Thank you!

azrsh added a commit to azrsh/node that referenced this issue Jan 14, 2024
In watch-mode, which executes short-lived scripts, there is a bug where
SIGINT or SIGTERM could not terminate the process because the signal
handler did not terminate. This change resolves this issue.

Fixes: nodejs#51466
@debadree25 debadree25 added the watch-mode Issues and PRs related to watch mode label Jan 15, 2024
@debadree25
Copy link
Member

Doing CtrlC is correctly terminating the process for me 🤔

@duncanchiu409
Copy link
Contributor

duncanchiu409 commented Jan 15, 2024

Ctrl C is terminating the process for me.

@azrsh
Copy link
Author

azrsh commented Jan 16, 2024

Oh! The --watch option was working fine in my environment as well.

Sorry, I had oversimplified the reproduction conditions and made a mistake. We can actually reproduce this by specifying the entry point script in --watch-path.

$ vim index.js
$ cat index.js
console.log("test");
$ node --watch-path index.js index.js
(node:1512205) ExperimentalWarning: Watch mode is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
test
Completed running 'test.js'
^C^C^C^C^C^C^C

Also, this behavior only seems to work on Linux. Could not reproduce on Mac.

@duncanchiu409
Copy link
Contributor

duncanchiu409 commented Jan 16, 2024

@azrsh Could re-produce the issue at v22.0.0-pre. However, I found your use of --watch-path is wrong. The correct command should be "node --watch-path=./ index.js". If you run my command, It should work as fine. Though, I could see a possible new issue on flag parsing. For example, not correct option on --watch-path should be stopped.

@duncanchiu409
Copy link
Contributor

@azrsh
Copy link
Author

azrsh commented Jan 17, 2024

In my environment, the command certainly works fine.

Does it mean that specifying a file instead of a directory in --watch-path is wrong?
I didn't find any such limitation written in the Node.js Command-line API documentation...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
watch-mode Issues and PRs related to watch mode
Projects
None yet
Development

No branches or pull requests

3 participants