Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for asynchronous execution #91

Closed
wants to merge 1 commit into from

Conversation

Binlogo
Copy link
Contributor

@Binlogo Binlogo commented Jul 15, 2024

This PR introduces asynchronous execution support.

Motivation

Sometimes, we'd like to run a command with a timeout, if the command stuck, user can be aware of it.

Proposal

  • Option 1: Directly use thread to implement a fn run_timeout.
  • Option 2: Support fn run_async / fn read_async and etc. to make use of tokio::time::timeout. ✅ (which this PR choose)

Change

  • Added run_async, read_async, read_stderr_async, and output_async methods to the Cmd struct under the async feature gating.
  • Added an example to demonstrates how to use tokio::time::timeout to apply a timeout to asynchronous command execution.

Relative issue

May move #76 forward?

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@matklad
Copy link
Owner

matklad commented Jul 15, 2024

I'd be against adding any kind of async runtime dependncy for this crate, even under a feature.

async xshell would be a useful crate. I can even see how it could be a better crate, as it'll allow you to easily spawn two tasks at the same time, and it'll also solve the "read stderr and stdout in lockstem" in much more elegant way. But that would be a different design, and it's best if it is worked on in a separate crate (btw, ashell name is free on crates.io ;). I'd gladly link to it from Readme, if it materializes.

At the same time, adding a timeout/deadline method, while preserving blocking API, would be great. I think it's OK to burn a thread just for that:

  • When we spawn a timeout-enabled process, we also spawn a timeout thread which also gets a reference to the Child's handle
  • That thread blocks in https://doc.rust-lang.org/stable/std/sync/mpsc/struct.Receiver.html#method.recv_timeout
  • if timeout elapses, it kills the process, which unblocks the main thread.
  • otherwise, it unblocks when the sender side of the channel is blocked (it's important that we don't leave hanging threads around)

@Binlogo
Copy link
Contributor Author

Binlogo commented Jul 16, 2024

This xshell is an excellent crate for building xtask, which I enjoy and have grown accustomed to. Therefore, I tend to add more features and continue using it.
Following your suggestion, I will close this PR and consider submitting a new one to introduce a timeout/deadline-specific extension without incorporating an async runtime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants