nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
107.8k stars 29.7k forks source link

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

Open azrsh opened 10 months ago

azrsh commented 10 months ago

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.

https://github.com/nodejs/node/blob/c25878d370c6d96abb382d9ee10d1838da4ec29d/lib/internal/main/watch_mode.js#L134

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.

https://github.com/nodejs/node/blob/c25878d370c6d96abb382d9ee10d1838da4ec29d/lib/internal/main/watch_mode.js#L74-L76

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

https://github.com/nodejs/node/blob/c25878d370c6d96abb382d9ee10d1838da4ec29d/lib/internal/main/watch_mode.js#L77-L79

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!

debadree25 commented 10 months ago

Doing CtrlC is correctly terminating the process for me 🤔

duncanchiu409 commented 10 months ago

Ctrl C is terminating the process for me.

azrsh commented 10 months ago

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 commented 10 months ago

@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 commented 10 months ago

Details on https://nodejs.org/docs/latest/api/cli.html#--watch-path

azrsh commented 10 months ago

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...