denoland / deno_task_shell

Cross-platform shell for deno task.
https://crates.io/crates/deno_task_shell
MIT License
106 stars 16 forks source link

Request for Thread-Safe Methods in deno_task_shell. #102

Closed avinashkurup closed 2 months ago

avinashkurup commented 9 months ago

Hi all, I am currently utilising deno_task_shell in a multi-threaded Rust application and have encountered situations where the need for thread safety within shell task execution is critical. Specifically, methods like execute_with_pipes are currently not guaranteed to be thread-safe, which can lead to issues in concurrent execution scenarios.

The ability to safely execute shell commands across multiple threads (we're using tokio at present) would greatly enhance the utility and robustness of deno_task_shell, especially for applications dealing with high concurrency or parallel processing needs.

Examples/Use Cases: Concurrent Command Execution: Running multiple shell commands in parallel where each command's input/output doesn't interfere with others. Shared State Management: Safely managing shared state or resources between different shell commands or tasks being executed concurrently. Suggested Enhancements: Implementation of thread-safe primitives like Arc, Mutex, RwLock, or usage of atomic types for shared state management within the crate's internal structure. Introduction of thread-safe variants of existing methods or entirely new methods designed for concurrent execution.

I believe that enhancing deno_task_shell with thread-safe capabilities would be greatly beneficial to the Rust community. I am looking forward to any discussion on this matter and am willing to assist in the implementation or testing phases if needed.

Thank you for considering this feature request.

Regards,

dsherret commented 8 months ago

I don't think there's any need to do this. For a multi-threaded tokio runtime, I believe you just need to run deno_task_shell in a LocalSet. It already supports stuff like executing commands on separate threads—the command can spawn a thread then just needs to communicate back to the shell on the original thread.

avinashkurup commented 8 months ago

Thank you @dsherret,

We are trying to do the same using this code,

                    let local_set = tokio::task::LocalSet::new();
                    // Run asynchronously using deno task shell.
                    let status = local_set
                        .run_until(execute_with_pipes(list, state, stdin, stdout, stderr))
                        .await;

                    let stderr = stderr_handle.await.unwrap();
                    let stdout = stdout_handle.await.unwrap();

but on execution the commands are running in a synchronous manner. Is there anyway to ensure that they run in a parallel manner. Any suggestions for use-cases for tokio or ways to achieve with deno_task_shell would be very helpful.

Regards,

dsherret commented 2 months ago

but on execution the commands are running in a synchronous manner

You should be able to spawn the work in a blocking thread if it's synchronous (tokio::task::spawn_blocking) or spawn a future in the original runtime using (I believe) tokio::task::spawn.

I don't want to make this code support an multi-threaded runtime because then it will run slower in Deno.