Summary
Cleanup, not a defect — I went looking for a bug here and did not find one, so this is about the comment and the discarded handle rather than observable behaviour.
setup_process_substitution in brush-core/src/interp.rs opens with:
// TODO(execute): Don't execute synchronously!
// Execute in a subshell.
but the function does not execute synchronously. It spawns the producer with tokio::spawn about twenty lines later and returns without awaiting it. The TODO appears to predate that and now points the reader at a problem that is not there.
What was checked
I assumed at first that the discarded result meant a failing producer would fail silently. It does not — errors travel out through the shell's normal stderr path before the Result is dropped, for both ordinary failures and genuine internal errors:
$ brush -c 'cat < <(definitely-not-a-command)'
error: command not found: definitely-not-a-command
$ brush -c 'cat < <(mapfile -C cb -c 1 -t x < /dev/null)'
error: mapfile: not yet implemented: mapfile -C/-c is not yet implemented
bash prints the equivalent. Lifetime behaviour also matches bash, both when the consumer exits before the producer finishes and when the shell would have to wait on a slow producer:
$ head -1 < <(printf "a\nb\nc\n"; sleep 2; echo late) # both: "a", immediate
$ cat < <(sleep 1; echo done) # both: "done", ~1.0s
So there is nothing user-visible to fix.
What is left
Two small things, both about maintainability:
-
The TODO is stale. It should be deleted, or rewritten to say what actually remains undone — as written it invites someone to "fix" an async spawn that is already async.
-
The JoinHandle is dropped, so the task is fully detached. let _ = ...execute(...).await; discards the Result, and the handle from tokio::spawn is never bound. A panic inside the producer is captured by the runtime in that JoinHandle and therefore silently discarded.
The second is worth a thought rather than an automatic change — detaching may well be the right call for a process substitution, whose whole point is to outlive the statement that created it. But if it is deliberate, the comment should say that, instead of the current "we intentionally don't block on its completion", which explains the non-blocking but not the unobserved failure.
Why it came up
While diagnosing #5. The failure mode there was that the producer task never ran at all, and because nothing anywhere observes that task, there was no signal of it — no error, no output, no timeout. The bug turned out to be elsewhere and the detachment was not the cause, but the absence of any observation point did make it harder to see what was happening.
Summary
Cleanup, not a defect — I went looking for a bug here and did not find one, so this is about the comment and the discarded handle rather than observable behaviour.
setup_process_substitutioninbrush-core/src/interp.rsopens with:but the function does not execute synchronously. It spawns the producer with
tokio::spawnabout twenty lines later and returns without awaiting it. The TODO appears to predate that and now points the reader at a problem that is not there.What was checked
I assumed at first that the discarded result meant a failing producer would fail silently. It does not — errors travel out through the shell's normal stderr path before the
Resultis dropped, for both ordinary failures and genuine internal errors:bash prints the equivalent. Lifetime behaviour also matches bash, both when the consumer exits before the producer finishes and when the shell would have to wait on a slow producer:
So there is nothing user-visible to fix.
What is left
Two small things, both about maintainability:
The TODO is stale. It should be deleted, or rewritten to say what actually remains undone — as written it invites someone to "fix" an async spawn that is already async.
The
JoinHandleis dropped, so the task is fully detached.let _ = ...execute(...).await;discards theResult, and the handle fromtokio::spawnis never bound. A panic inside the producer is captured by the runtime in thatJoinHandleand therefore silently discarded.The second is worth a thought rather than an automatic change — detaching may well be the right call for a process substitution, whose whole point is to outlive the statement that created it. But if it is deliberate, the comment should say that, instead of the current "we intentionally don't block on its completion", which explains the non-blocking but not the unobserved failure.
Why it came up
While diagnosing #5. The failure mode there was that the producer task never ran at all, and because nothing anywhere observes that task, there was no signal of it — no error, no output, no timeout. The bug turned out to be elsewhere and the detachment was not the cause, but the absence of any observation point did make it harder to see what was happening.