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

builtin/custom command may become blocking when pipe is full #22

Closed
rust-shell-script opened this issue Apr 4, 2021 · 0 comments
Closed

Comments

@rust-shell-script
Copy link
Owner

Right now for builtin/custom commands, we just call the registered functions to get the results instead of spawning separate processes. However, when they are used in pipes, it could become blocking when the pipe is full:

code:

rust_cmd_lib git:(master) ✗ cat examples/one.rs 
use cmd_lib::*;

fn main() -> CmdResult {
    init_builtin_logger();
    use_builtin_cmd!(cat);    // remove this line, there will be no blocking
    let big_file_content = "a".repeat(64 * 4096 * 2);
    std::fs::write("/tmp/big_file", big_file_content)?;
    run_cmd!(cat /tmp/big_file | wc)?;
    Ok(())
}

output:

rust_cmd_lib git:(master) ✗ CMD_LIB_DEBUG=1 cargo run --example one
    Finished dev [unoptimized + debuginfo] target(s) in 0.02s
     Running `target/debug/examples/one`
DEBUG - Running ["cat", "/tmp/big_file"] | ["wc"] ...
---> blocking here

We can fix the above bug by launching threads for each builtin/custom commands, however, it seems overkill for other builtin commands like echo/true/info/error/debug/..., and it would also complicate the current code. Need to figure out a way to solve this blocking issue more gracefully.

@tao-guo tao-guo closed this as completed in 79cf45f Apr 5, 2021
tao-guo added a commit that referenced this issue Apr 5, 2021
there will be 3 child handles:
1. normal process child handle for external commands
2. sync function handle for builtin/custom commands without piping
3. thread spawning handle for builtin/custom pipe-out commands

fix #22
tao-guo added a commit that referenced this issue Apr 5, 2021
there will be 3 child handles:
1. normal process child handle for external commands
2. sync function handle for builtin/custom commands without piping
3. thread spawning handle for builtin/custom pipe-out commands

fix #22
tao-guo added a commit that referenced this issue Apr 5, 2021
change thread handle return type from () to CmdResult
fix #22
tao-guo added a commit that referenced this issue Apr 5, 2021
change thread handle return type from () to CmdResult
fix #22
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

No branches or pull requests

1 participant