Skip to content

Commit

Permalink
Add forking workaround for block stdout redir (re: e373e8c)
Browse files Browse the repository at this point in the history
Reproducer:

    ok=$({ true >&2; } >&2; echo OK); echo $ok

This should output "OK" but currently outputs nothing.

Looks like the sh_subfork() workaround must also be implemented for
the TSETIO case in sh_exec(), as that is where redirections for
blocks and compound commands are handled. This avoids inconsistent
behaviour that happens if sh_subfork() is called later for the
inside redirection.

src/cmd/ksh93/sh/xec.c: sh_exec(): case TSETIO:
- Also fork if standard output redirections attached to blocks are
  executed within a non-subshare command substitution.

Thanks to Vincent Mihalkovič (@vmihalko) for the report.
Resolves: #784
  • Loading branch information
McDutchie committed Oct 31, 2024
1 parent afa0649 commit 5def439
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 9 deletions.
8 changes: 8 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,14 @@ This documents significant changes in the dev branch of ksh 93u+m.
For full details, see the git log at: https://github.com/ksh93/ksh
Uppercase BUG_* IDs are shell bug IDs as used by the Modernish shell library.

2024-10-31:

- Fixed a regression that caused a standard output redirection within a block
construct to cause inconsistent behaviour or a hang if: (a) standard output
was also redirected on the block itself, and (b) the entire block was
executed from within a command substitution. Bug introduced on 2022-01-11.
Reproducer (should output "OK"): ok=$({ true >&2; } >&2; echo OK); echo $ok

2024-10-27:

- Fixed a bug that crashed the shell upon attempting to replace the shell
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/include/version.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

#define SH_RELEASE_FORK "93u+m" /* only change if you develop a new ksh93 fork */
#define SH_RELEASE_SVER "1.1.0-alpha" /* semantic version number: https://semver.org */
#define SH_RELEASE_DATE "2024-10-27" /* must be in this format for $((.sh.version)) */
#define SH_RELEASE_DATE "2024-10-31" /* must be in this format for $((.sh.version)) */
#define SH_RELEASE_CPYR "(c) 2020-2024 Contributors to ksh " SH_RELEASE_FORK

/* Scripts sometimes field-split ${.sh.version}, so don't change amount of whitespace. */
Expand Down
15 changes: 8 additions & 7 deletions src/cmd/ksh93/sh/xec.c
Original file line number Diff line number Diff line change
Expand Up @@ -1712,21 +1712,22 @@ int sh_exec(const Shnode_t *t, int flags)
int jmpval, waitall = 0;
int simple = (t->fork.forktre->tre.tretyp&COMMSK)==TCOM;
struct checkpt *buffp = stkalloc(sh.stk,sizeof(struct checkpt));
if(sh.subshell && !sh.subshare && t->fork.forkio)
if(sh.subshell && !sh.subshare)
{
/* Subshell forking workaround for https://github.com/ksh93/ksh/issues/161
* Check each redirection for >&- or <&-
/* Subshell forking workaround for:
* https://github.com/ksh93/ksh/issues/161 (check each redirection for >&- or <&-)
* https://github.com/ksh93/ksh/issues/784 (check for stdout in a command substitution)
* TODO: find the elusive real fix */
struct ionod *i = t->fork.forkio;
do
struct ionod *i;
for (i = t->fork.forkio; i; i = i->ionxt)
{
if((i->iofile & ~(IOUFD|IOPUT)) == (IOMOV|IORAW) && !strcmp(i->ioname,"-"))
unsigned f = i->iofile;
if ((f & ~(IOUFD|IOPUT))==(IOMOV|IORAW) && !strcmp(i->ioname,"-") || (f & IOUFD)==1 && sh.comsub)
{
sh_subfork();
break;
}
}
while(i = i->ionxt);
}
sh_pushcontext(buffp,SH_JMPIO);
if(type&FPIN)
Expand Down
32 changes: 31 additions & 1 deletion src/cmd/ksh93/tests/io.sh
Original file line number Diff line number Diff line change
Expand Up @@ -936,7 +936,6 @@ fi # can_close_stdout
# ======
# Command substitution hangs, writing infinite zero bytes, when redirecting standard output on a built-in that forks
# https://github.com/ksh93/ksh/issues/416
exp='line'
"$SHELL" -c 'echo "$(ulimit -t unlimited >/dev/null 2>&1; echo "ok $$")"' >out 2>&1 &
pid=$!
(sleep 1; kill -9 "$pid") 2>/dev/null &
Expand All @@ -947,6 +946,37 @@ then kill "$!" # the sleep process
else err_exit "comsub hangs after fork with stdout redirection"
fi
# https://github.com/ksh93/ksh/issues/416#issuecomment-1008866883
exp='line'
"$SHELL" -c 'alias foo=bar; echo $(alias foo >/dev/null; echo "$1")' "$0" "$exp" >out 2>&1 &
pid=$!
(sleep 1; kill -9 "$pid") 2>/dev/null &
if wait "$pid" 2>/dev/null
then kill "$!" # the sleep process
[[ $(<out) == "$exp" ]] || err_exit "comsub fails after stdout redirection" \
"(expected '$exp', got '$(<out)')"
else err_exit "comsub hangs after stdout redirection"
fi
# same bug for compound/block commands: https://github.com/ksh93/ksh/issues/784
exp=$'funA\nA'
"$SHELL" -c '
BugFunction() {
{ echo "funA" >&2; } >&2
echo A
}
Result=$(BugFunction)
echo $Result
' >out 2>&1 &
pid=$!
(sleep 1; kill -9 "$pid") 2>/dev/null &
if wait "$pid" 2>/dev/null
then kill "$!" # the sleep process
[[ $(<out) == "$exp" ]] || err_exit "double redirection in command substitution" \
"(expected $(printf %q "$exp"), got $(printf %q "$(<out)"))"
else err_exit "double redirection in command substitution causes shell hang"
fi
# ======
# https://github.com/ksh93/ksh/issues/161
got=$(
Expand Down

0 comments on commit 5def439

Please sign in to comment.