thefrontside / ctrlc-windows

Send a CTRL-C event to a Windows console application
9 stars 2 forks source link

✨Use "killer" strategy, so parent process doesn't need to get its hands dirty 🔪 #2

Closed cowboyd closed 4 years ago

cowboyd commented 4 years ago

We were having race conditions where the console of the parent process console was getting into an inconsistent state. We think this was because we were calling FreeConsole, AttachConsole, FreeConsole, and AttachConsole again.

This avoids having to make the parent process detach from its console temporarily by spawning a "killer" process whose only purpose is to throw away its own console, attach to the target process's console and then send a ctrl-c event to it. Then the "killer" process can die itself and we never have to worry about the parent process.

We still do have to temporarily disable ctrl-c handling in the parent process since it is possible that it shares a console with the target process.

Some caveats:

jnicklas commented 4 years ago

Why are the yields necessary?

As for spam from cmd.exe, you could silence that by piping its stdout into null. https://doc.rust-lang.org/std/process/struct.Command.html#method.stdout

We could also build our own binary which does nothing. Maybe that complicates things too much though?

std::process::Command - Rust
API documentation for the Rust `Command` struct in crate `std`.
cowboyd commented 4 years ago

Why are the yields necessary?

The yields were necessary because without them there was some really undefined behavior. It felt like every time I ran the test it was a different kind of failure. I tried taking some of them out, but it would either cause mocha to hang, kill mocha, exit, but never print any result, or some combination of the above but with the console output mucked with external characters.

cowboyd commented 4 years ago

Hmm, piping to null almost worked. there's still some "Terminate Batch Job (Y/n)" weirdness to track down after the weekend

> C:\Users\cowboyd\ctrlc-windows>yarn test
yarn run v1.22.10
$ mocha -r ts-node/register "test/**/*.test.ts"

  ctrlc
    on a process that does not exist
      √ is is just a no-op
    on a running process
Terminate batch job (Y/N)?       √ causes that process to exit

  2 passing (203ms)
changeset-bot[bot] commented 4 years ago

⚠️ No Changeset found

Latest commit: 0f127aa4577ff8cd4839a4b634824acec15a7f24

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

cowboyd commented 4 years ago

@jbolda I went ahead and removed the unix compilation entirely