script hangs terminal

[dakom]

Describe the bug
Running a task that launches npx webpack-dev-server causes a hang in windows when exiting

To Reproduce

1. Install node, npm, etc.
2. Create a minimal webpack project (and install webpack-dev-server)
3. Start the webpack-dev-server via npx directly:

npx webpack-dev-server --config webpack.config.js

4. Control-C to break out
5. No problem so far.. can type on the console, task manager looks clean - all good
6. Now try via cargo make...
7. Create following in Makefile.toml

[tasks.webpack-development-server]
script = ["npx webpack-dev-server --config webpack.config.js"]

8. Launch it via cargo make webpack-development-server
9. Starts fine
10. Control-C to exit
11. Appears to exit but then terminal is messed up - can't enter text
12. Also task manager shows additional consoles (this is after hitting control-c in the terminal but before closing the window entirely):

Error Stack

None - it's just that the terminal becomes unusable

[sagiegurari]

sorry, but i won't be able to investigate this one as i'm not going to install all that and i do not develop on windows at all.

[dakom]

Understood. I can work around it in the meantime by launching cargo make from npm instead of the other way around.

If you have any tips to like where in the source code to look at that might cause a hang, maybe I'll take a look down the road. Not a high priority for me either though since the workaround is fine

[sagiegurari]

scripts are executed in the command.rs and it is using another library run_script (also mine) which creates a script file and executes it.

[dakom]

started doing a bit of sleuthing and was able to recreate the bug in a local project using just run_script (not cargo make) ... will open an issue there just for the sake of keeping track

[dakom]

OK as noted over in run_script - the issue isn't windows or npx, but rather what I'm launching (webpack-dev-server) is a server that just hangs around and expects to be killed with control-c

Can there be an option in cargo-make to call run_script::spawn() instead of run_script::run()?

In this case, cargo-make would need to stay active (like in watch mode I guess) and then kill() the child when its own process is killed.

The output from the command can't easily be captured this way - but that's fine. It gets shown on the terminal as usual anyway

[dakom]

Yup! The same problem happens with any other command that hangs around.

For example, assuming watchexec is installed, this will demonstrate the problem:

[tasks.foo]
script = ["watchexec \"echo foo\""]

cargo make foo will execute it and everything is fine - but ctrl-c then does wacky stuff and can't type in the terminal (curious what the effect is on linux!)

[sagiegurari]

i use cargo watch on linux and cntl c works ok there.
you can get a linux box via gitpod. its really easy

[dakom]

on windows internal watch works fine, but external does not. for example, script1 and script2 hang, but script3 is totally fine:

[tasks.script1]
script = ["watchexec \"echo foo\""]

[tasks.script2]
script = ["cargo watch -s \"echo foo\""]

[tasks.script3]
script = ["echo foo"]
watch = true

However you're launching cargo watch internally... can that be used to launch arbitrary scripts the same way?

[dakom]

so it's not just the internal watch that works fine... commands are okay too:

[tasks.command1]
command = "watchexec"
args = ["echo", "foo"]

[sagiegurari]

thanks for the investigation.
i'm guessing in most cases it should be ok then as cargo make usually runs tasks and ends and doesn't usually used for running forever running processes.
if it does, its usually with built in watch=true.
having said that your use case is 100% valid and is a combination of

1. running a forever process
2. having to run it as a script and not as a command as the thing is a cmd script and not an executable.

to workaround that what you can do is

[tasks.npx.windows]
command = "cmd.exe"
args = [ "/C", "npx", .....]

which makes cargo make run it as a command.
and for linux/mac use the tasks.npm to do something similar with bash.

[dakom]

Thanks, will keep that in mind as another workaround :)

But... I found the solution! though I don't understand enough about Command to know if it's valid...

In the run_script crate - the fix is changing this line in create_command_builder()

from this:

command.stdin(Stdio::inherit());

to this:

command.stdin(Stdio::piped());

The way I ended up there is looking at command, it calls Command::output() instead of Command::spawn() - and Command::output() does this:

pub fn output(&mut self) -> io::Result<Output> {
        self.inner.spawn(imp::Stdio::MakePipe, false).map(Child::from_inner)
            .and_then(|p| p.wait_with_output())

(first arg is pipe, not inherit)

Is this okay - for scripts to also use pipe() instead of inherit() for stdin? (I don't know what all the implications are here... just kinda saying things without real understanding. sorry!)

[dakom]

Another interesting data point... for some reason the command = "cmd.exe" workaround does not work.... still freezes for some reason. There's no cache being generated or something right?

Good news is though that I tested on a local fork the above suggested change to run_script and after doing that, this works just fine, so I imagine it'd carry over to cargo-make too (assuming the suggested change actually makes sense!):

let script_lines = r#"
  npx webpack-dev-server --config webpack.dev.js
"#;

run_script::run(script_lines, &Vec::new(), &ScriptOptions::new());

[sagiegurari]

cool. so open a pr for run script please.
I'll run some tests and if all ok, I'll push it.
thanks a lot for all the investigations

[dakom]

Done. Goodnight :D

[mogenson]

Hello, I think I may be seeing this same issue on cargo-make 0.26.2 from crates.io. This version should include run_script 0.6 with the chage stdin to piped commit. I'm on Linux x86_64.

I've tried a few of the above suggestions to create a task that launches GDB to debug a Rust program. You can see a small dummy project here: https://github.com/mogenson/cargo-make-ctrl-c

GDB uses CtrlC to halt the execution of a program. In a normal shell, such as sh -c "rust-gdb ./target/debug/cargo-make-ctrl-c", CtrlC is handled by GDB and the shell does not close until GDB exits. When run as a cargo-make task, it appears that the CtrlC signal propagates up, is caught by cargo-make which terminates, and leaves GDB in a running but orphaned state.

Is it possible to run a command or script in a new session, attached to the same TTY, and wait until the command or script has finished? Something similar to the setsid Linux command.

[mogenson]

By the way, when using the setsid command in any tasks, they fail with setsid: failed to set the controlling terminal: Operation not permitted

[sagiegurari]

the changes in run script are not used by cargo make. i need to think of how to enable them when needed, probably some flag or something similar

[mogenson]

I found a workaround by abusing the script command. script will swallow CtrlC and does not require super user permissions like setsid.

I've updated my dummy project with an example.

I'll keep an eye out for a cargo-make update with a flag or option, since that will be a lot less hacky.

[sagiegurari]

@dakom @mogenson I updated the 0.26.3 development branch with the fix (i hope).
its using the new run_script flag.
to enable it you need to set the env var CARGO_MAKE_SCRIPT_FORCE_PIPE_STDIN to true.
for example:

[tasks.force-pipe-stdin]
env = { CARGO_MAKE_SCRIPT_FORCE_PIPE_STDIN = true }
script = [
'''
    echo start
    sleep 20
    echo end
'''
]

can you please validate this fix and tell me if it solves it?

[mogenson]

I installed the 0.26.3 branch. Unfortunately, with your above force-pipe-stdin task I can still Ctrlc quit the script and cargo-make.

If I use the environment flag in my GDB example, the task ends immediatly after launching GDB. It does not wait for the process to finish.

[tasks.debug-5]
dependencies = ["build"]
env = { CARGO_MAKE_SCRIPT_FORCE_PIPE_STDIN = true }
script = ["rust-gdb ./target/debug/cargo-make-ctrl-c"]

[sagiegurari]

@mogenson thanks a lot for the feedback.
maybe you can play around with the change i did to find the right out/in option and if you do, we will expose those as some env to enable to workaround this.

@dakom what about your use case? does this resolve it?

[dakom]

Can't check today but will try tomorrow... in the meantime, thanks :)

[dakom]

sorry about the delay...

good news, this does fix it - thanks so much!!

little test repo here: https://github.com/dakom/cargo-make-stdio-test

[sagiegurari]

@dakom thanks a lot for checking.

@mogenson I'll try to check gdb flow as well and will update here

[sagiegurari]

@mogenson i'm not using my fix in your use case.
in the gdb console, i simply type quit to exit it.
it looks like thats the proper way to kill it so i'm not sure there is something needed to do in cargo-make

[mogenson]

@sagiegurari this is the workflow I've been testing:

• clone https://github.com/mogenson/cargo-make-ctrl-c.git
• run cargo make debug-1 (or debug-2 or debug-3)
• type run in the gdb prompt to start test program execution
• press ctrl-c to break execution
• GDB may catch sigint or cargo-make may exit
• if you press ctrl-c again, cargo-make will definitely exit, and you'll be back at the shell prompt
• after a moment GDB will exit and print to the prompt:

A debugging session is active.
	Inferior 1 [process 271902] will be killed.
Quit anyway? (y or n)
EOF [assumed Y]

For comparison, if you run GDB without cargo-make via gdb ./target/debug/cargo-make-ctrl-c you can press ctrl-c all day and GDB will not quit.

[sagiegurari]

ok thanks for the detailed explanation. will try it.

[mogenson]

If GDB is kind of a niche case, I've tried the following cargo make task:

[tasks.cat]
#env = { CARGO_MAKE_SCRIPT_FORCE_PIPE_STDIN = true }
script = [ "echo start; cat; echo end" ]

As-is, pressing ctrl-c to exit cat also exits cargo-make and you don't see the last echo. With the env option uncommented, cat does not wait for stdin and the task finishes immediately.

[sagiegurari]

I'm going to close this issue as original problem was resolved.
@mogenson I have opened a new issue specifically for your flow in #374