- Notifications
You must be signed in to change notification settings - Fork 27
Description
This was probably not much of a problem previously because wait3() would clean up errant processes like this, but after migrating to waitpid() in #128 it's more obvious. See the following:
; %read << EOF hello EOF ; echo <=wait # surprising 0 ; echo <=wait # not surprising wait: No child process My expectation would be to get a No child process exception on the first wait.
There's nothing obvious in the blame layer to suggest this is intentional. However, I think it's interesting that $&here is written like the other "normal" redirections (using a REDIR(here)) but it also performs a pipefork() like the (not-REDIR()-using) primitives $&pipe, $&readfrom/$&writeto, and $&backquote, all of which do wait for their child processes. Maybe the authors back in the day hemmed and hawed about implementing $&here via a fork, since it's not strictly necessary?
The simple/narrow fix is to, in PRIM(here) save the return value from redir(), ewait() for the writing process to finish, and then return it (plus the appropriate exception-handling-wrapping).
However, if we wanted, we could also get rid of the fork sometimes or always. Other shells seem to use a few different strategies for heredocs:
- bash directly tests pipe buffer sizes at build time, and uses a pipe for heredocs smaller than the tested buffer size, and a temporary file for larger ones.
- zsh (based a quick strace) seems to always use a temporary file for heredocs.
- dash uses a simple-to-guess minimum pipe size (
PIPE_BUF), uses a pipe for smaller heredocs, and forks for larger ones. - rakitzis rc, still, always forks off a pipe writer.
I'm somewhat in favor of following dash's example here. It seems relatively simple while still making the common short-heredoc case (heredoc length < the system's PIPE_BUF, minimum 512 per POSIX but usually 4096) fast. I don't actually have a strong intuition on whether a fork or creating/deleting a temporary file is faster on most systems, so I don't have a strong urge to change the "longer" case. Plus, if users really want a tmpfile-based %here, they can define their own version something like this:
fn %here fd input cmd { let (f = /tmp/es.here.$pid) unwind-protect { echo -n $input > $f %open $fd $f $cmd } { rm -f $f } }