fix(@angular/cli): stop ng serve on first Ctrl+C received

[devoto13]

This fixes a regression introduced in 1ce4db6. Tested manually on macOS:

Before:

$ ng serve --aot
-> Ctrl+C // nothing seem to happen
-> Ctrl+C // ng serve exits

After:

$ ng serve --aot
-> Ctrl+C // ng serve exits

Fixes #9647

[filipesilva]

I did see something on the sort, with --aot only. But I don't know why it happens.

We can't do process.exit() though, as the process might need to exit some other way, or remain running because it's being composed with other code.

This needs to be investigated a bit further... I don't know if there is a bug with the current changes, or if there's something retaining the ngc compiler up.

[devoto13]

@filipesilva Fare enough. I'll take a look tomorrow and see if I can figure out what's the real cause of the issue.

[victornoel]

@filipesilva for the record, please take a look at #9647 (and other related bugs there) because it is not just a question of hitting ctrl+C twice to close the process: when used in conjunction with yarn or npm the process stays open and it makes angular-cli simply impossible to use.
Also it is not only with --aot, it happens without it too.

[devoto13]

@victornoel The issue being fixed here is the root cause of all effects described in the linked issue.

Now I'll be speculating, because I didn't get time to investigate it properly. Let's say we're running ng serve directly as described above.

When you hit Ctrl+C OS will send SIGINT to the process (kindly asking it to stop). CLI subscribes to signal in 2 places: here and here. My theory is that Node only passes signal to the last registered callback, which is in compiler plugin in this case. So when first SIGINT is received it will be handled by compiler plugin and stop type checker. But the main CLI process will keep running as it didn't receive the SIGINT. When you you hit Ctrl+C second time it will reach main process and stop it (because compiler plugin subscribed using process.once and is not listening anymore).

When you run through yarn start it will pass only first SIGINT to the ng serve process and exit itself. So now only type checker is stopped, but ng serve keeps running in the background as yarn stopped as well.

The reason why it works correctly with npm run start is probably because NPM makes sure to kill all processes subtree before exiting itself.

Again these are just assumptions. I didn't have time to properly check them.

Okey, I'm just tired at the end of the day. Obviously if AngularCompilerPlugin subscribed to SIGINT and did not stop itself when received it process won't stop >.< Two lines repro below:

process.stdin.resume();
process.once('SIGINT', () => console.log('Received SIGINT and ignored it once.'));

@filipesilva I think the right fix is to not handle SIGINT in the compiler plugin, but handle only exit (normal process stop) and uncaughtException (for crashes). I'll update a PR.

[devoto13]

FWIW if I remove all cleanup logic it correctly terminates child type checker process on macOS, when parent process is terminated. Can somebody test it on Windows as well?

[filipesilva]

@devoto13 I think your fix is good and tested it in a couple of different systems.

Running both npm start and ng serve and then killing them via ctrl+c resulted in no leftover processes in both win10+node8.9 and osx+node6 (thanks @Foxandxss!).

I have to ask you to still keep treeKill though, as killing processes on windows is harder than on OSX/Linux, and that's what treeKill is there for.

Other than that, LGTM. Do you have time to make those changes today? I would like to get this fix into the next release.

[devoto13]

@filipesilva Yes, I'll do it right away!

[filipesilva]

Awesome work @devoto13, I'll add it to the list for the next release!

[maxime1992]

https://twitter.com/sindresorhus/status/974343649600323584 🎉

[filipesilva]

@maxime1992 oh that's nice!

[zakhenry]

‘killall ng’ works well too :)

On Thu, 15 Mar 2018 at 10:41 pm, Filipe Silva ***@***.***> wrote: @maxime1992 <https://github.com/maxime1992> oh that's nice! — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#9668 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAsCad-vs8rCh9p4kEStFnTtavMhu61Jks5teu4JgaJpZM4SJbld> .

-- Zak Henry

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}