fix(cli.rs): terminate the beforeDevCommand, closes #2794

[lucasfernog]

What kind of change does this PR introduce? (check at least one)

• Bugfix
• Feature
• Docs
• New Binding Issue #___
• Code style update
• Refactor
• Build-related changes
• Other, please describe:

Does this PR introduce a breaking change? (check one)

• Yes. Issue #___
• No

The PR fulfills these requirements:

• When resolving a specific issue, it's referenced in the PR's title (e.g. fix: #xxx[,#xxx], where "xxx" is the issue number)
• A change file is added if any packages will require a version bump due to this PR per the instructions in the readme.

If adding a new feature, the PR's description includes:

• A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information:

[amrbashir]

steps I took to produce the issue or at least one of them:

1. create a svelte project using create-tauri-app
2. run tauri dev
3. wait only until the svelte devServer is up and before the tauri window is shown then use Ctrl+C to terminate the process.

results:

• before this PR, the tauri dev process exists but it doesn't terminate the devServer of svelte and you can't use this terminal session anymore.
• after this PR, using Ctrl+C doesn't terminate the process, it instead hangs, sometimes indefinitely and sometimes until the tauri window appears but when that happens the devServer of svelte is killed.

[probablykasper]

@amrbashir Could be this:

• SIGTERM has no effect sveltejs/template#243

[amrbashir]

yeah I think it is related, I talked about it in discord before.

[amrbashir]

This issue is driving me crazy, I managed to reproduce it with vite in #2998. Same steps, wait for vite devServer to run, press Ctrl+C, tauri devprocess is terminated but not vite process and the terminal session is unusable.

[amrbashir]

A bit of an update, in svelte official template, I found out that they are using ignore instead of inherit https://github.com/sveltejs/template/blob/4fd09c2a191b017ca9eb9cb9921fa873e95f44b4/rollup.config.js#L21

Switching it to inherit doesn't fix the whole issue but at least Ctrl+C terminates it properly.

[lucasfernog]

@probablykasper can you still reproduce this issue? I couldn't after a lot of attempts.

[FabianLars]

Seems to work wonderfully on windows 🥳

[probablykasper]

@lucasfernog Yes. Tried running create-react-app with the Svelte template, then ran npm run tauri dev and closed the window it opened. The npm run tauri dev command exits, but the node process is left running

[lucasfernog]

I tried the same steps but I couldn't reproduce it.

[probablykasper]

Unable to reproduce it both with this PR and without?

[probablykasper]

Screen recording:
https://user-images.githubusercontent.com/11315492/159995391-4316158f-be50-4f8b-b986-fe4cead80886.mp4

[lucasfernog]

Looks like we need a better script to kill child processes on Unix (currently kill -TERM -P {pid}).

[probablykasper]

I don't think there's a better way to do it than that. As long as SIGTERM is actually called on the child process, then it should be a bug in the child process if it's not doing what it's told

Best bet might be to instead use a different Svelte template in create-tauri-app

[lucasfernog]

Yeah but I think we can fix this on our end, I'm testing atm.

[lucasfernog]

@probablykasper try again, it works now!

[probablykasper]

The change is you're calling 9 (SIGKILL) instead of SIGTERM, right? I'm not sure that's a great idea :/

[lucasfernog]

Let me try with SIGTERM again

[lucasfernog]

hmm I think there's no other way, either we leave Svelte users with this super annoying bug or we use SIGKILL

[probablykasper]

I wouldn't want to risk using SIGKILL, could have some rough consequences since it takes away processes' ability to shut down properly :/

As Amr pointed out, the Svelte template might just be spawning it's child process incorrectly

[lucasfernog]

i'll just revert it, maybe there's a place for us to document it