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

ksh: command substitution doesn't include the output of non-waited-for child processes #1479

Open
stephane-chazelas opened this issue May 6, 2020 · 1 comment

Comments

@stephane-chazelas
Copy link

stephane-chazelas commented May 6, 2020

That's a regression since (introduced after) ksh93u+. Here tested with ksh2020 on Ubuntu 20.04.

$ echo "<$(sh -c '(sleep 1; echo after) &'; echo now)>"
<now>

That's missing after.

Note that it also applies to the ${ cmd; } form of command substitution.

It looks like that's because ksh switched to using temporary files instead of pipes to retrieve the output of commands in command substitutions.

As a result it cannot know when the spawned commands have finished writing.

$ ksh93u+ -c 'echo "$(ls -lL /dev/stdout)"'
prw------- 1 chazelas chazelas 0 May  6 16:31 /dev/stdout
$ ksh2020 -c 'echo "$(ls -lL /dev/stdout)"'
-rw------- 0 chazelas chazelas 0 May  6 16:31 /dev/stdout

When then are child processes started beforehand within the cmdsubst via &, then it seems ksh implements a sort of work around to the issue, so that's one way to work around the issue:

$ ksh2020 -c 'echo "<$( : & (sh -c "sleep 1; echo after" &);  echo now)>"'
<now
after>

By adding that : &, I now correctly get the output of all commands in the command substitution.

That seems to be done by opening a pipe (not used) on a high fd, which the parent uses to determine when the child processes are gone:

$ ksh2020 -c 'echo "$(ls -l /dev/fd/)"'
total 0
lrwx------ 1 chazelas chazelas 64 May  6 16:37 0 -> /dev/pts/7
lrwx------ 1 chazelas chazelas 64 May  6 16:37 1 -> /tmp/sf.XXp6j2iu (deleted)
lrwx------ 1 chazelas chazelas 64 May  6 16:37 2 -> /dev/pts/7
lr-x------ 1 chazelas chazelas 64 May  6 16:37 3 -> /proc/495315/fd
$ ksh2020 -c 'echo "$(:& ls -l /dev/fd/)"'
total 0
lrwx------ 1 chazelas chazelas 64 May  6 16:37 0 -> /dev/pts/7
lrwx------ 1 chazelas chazelas 64 May  6 16:37 1 -> /tmp/sf.XX2H7N7G (deleted)
l-wx------ 1 chazelas chazelas 64 May  6 16:37 13 -> pipe:[421750]
lrwx------ 1 chazelas chazelas 64 May  6 16:37 2 -> /dev/pts/7
lr-x------ 1 chazelas chazelas 64 May  6 16:37 3 -> /proc/495410/fd

But that is brittle and not correct as we're now waiting for all the processes (that haven't closed their fd 13 here), not for the command to finish outputting. In output=$(cmd & start-daemon), we could end-up waiting for the daemon to die, if that daemon doesn't close its fd 13 (or whatever fd ksh uses for the synchronising pipe). Example:

$ time ksh2020 -c 'echo "$(sleep 2 > /dev/null & echo x)"'
x
ksh2020 -c 'echo "$(sleep 2 > /dev/null & echo x)"'  0.00s user 0.01s system 0% cpu 2.007 total

ksh ended up waiting for sleep even though it was started in background with its stdout redirected to /dev/null here.

It's also quite buggy:

$ ksh2020 -c 'echo "<$( ( (sleep 1; echo after) &);  echo now)>"'
<now>
$ <after>

the echo "<$(...)>" was run twice! Wrong fork()ing it would seem.

That also means that output of command substitutions now end up being stored both on filesystem and memory, so things like secret=$(generate secret) are less safe.

$ ksh -c 'secret=$(echo secret; /bin/sleep 20)' &
[1] 506720
$ ps
    PID TTY          TIME CMD
 282660 pts/7    00:00:04 zsh
 506720 pts/7    00:00:00 ksh
 506721 pts/7    00:00:00 sleep
 506722 pts/7    00:00:00 ps
$ cat /proc/506721/fd/1
secret

(see also how a fd opened on a file containing the secret was leaked to sleep).

@jghub
Copy link

jghub commented May 6, 2020

I have these remarks:

  1. I can confirm it is a regression introduced after ksh93u+ (which does the right thing here AFAICS in a quick test)

  2. the present repo is no longer under active development. please read up on background in Rewinding this repo and encouraging community #1466 and further issues referenced therein.

  3. if you are interested in ksh93 and encounter bugs in ksh93u+ (rather than ksh2020) you might consider to report them at

https://github.com/ksh-community/ksh

which intends to maintain ksh93 in the future.

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

2 participants