Demonstrate best practice for feeding stdin of a child processes #82943
Conversation
|
(rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
@kornelski I think explaining this issue and providing an example is a good idea. So is demonstrating Would you consider adding a note explaining when this is needed, along the lines of: "This is an issue when running any program that doesn't have a stated guarantee that it reads its entire stdin before writing more than a pipe buffer's worth of output, if you want to feed it more than a pipe buffer's worth of input. The size of a pipe buffer varies on different targets." Also, one other issue (which may or may not be possible to simply address here): it'd be nice if somewhere we had a good example of error handling for threads using |
|
This PR addresses a very common problem where people deadlock their program and can't figure out why. Without derailing this specific PR: Should we have a |
It's possible to create a deadlock with stdin/stdout I/O on a single thread: * the child process may fill its stdout buffer, and have to wait for the parent process to read it, * but the parent process may be waiting until its stdin write finishes before reading the stdout. Therefore, the parent process should use separate threads for writing and reading.
|
I've added more explanations. I agree that changing |
|
@bors r+ rollup |
|
📌 Commit ce2d95c has been approved by |
bors
added
S-waiting-on-bors
|
@kornelski wrote:
Looks great. Merging.
Thank you! |
Rollup of 10 pull requests Successful merges: - rust-lang#81465 (Add documentation about formatting `Duration` values) - rust-lang#82121 (Implement Extend and FromIterator for OsString) - rust-lang#82617 (Document `everybody_loops`) - rust-lang#82789 (Get with field index from pattern slice instead of directly indexing) - rust-lang#82798 (Rename `rustdoc` to `rustdoc::all`) - rust-lang#82804 (std: Fix a bug on the wasm32-wasi target opening files) - rust-lang#82943 (Demonstrate best practice for feeding stdin of a child processes) - rust-lang#83066 (Add `reverse` search alias for Iterator::rev()) - rust-lang#83070 (Update cargo) - rust-lang#83081 (Fix panic message of `assert_failed_inner`) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Documentation change.
It's possible to create a deadlock with stdin/stdout I/O on a single thread:
Therefore, the parent process should use separate threads for writing and reading.
These examples are not deadlocking in practice, because they use short strings, but I think it's better to demonstrate code that works even for long writes. The problem is non-obvious and tricky to debug (it seems that even libstd has a similar issue: #45572).
This also demonstrates how to use stdio with threads: it's not obvious that
.take()can be used to avoid fighting with the borrow checker.I've checked that the modified examples run fine.