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

EXIT traps don't work in forked subshells #1452

Open
McDutchie opened this issue Dec 21, 2019 · 5 comments
Open

EXIT traps don't work in forked subshells #1452

McDutchie opened this issue Dec 21, 2019 · 5 comments
Labels

Comments

@McDutchie
Copy link
Contributor

Description of problem:
When a subshell forks (see #480 for discussion), EXIT traps do not work.

Ksh version:

  • The "stable" release Version A 2020.0.0
  • current git: 2020.0.0-beta1-190-gc6757a29

How reproducible:
Always

Steps to reproduce:
Test script:

(trap 'echo EXIT trap 1 okay' EXIT)
(ulimit -t unlimited; trap 'echo EXIT trap 2 okay' EXIT)

Actual results:
Only "EXIT trap 1 okay'" is printed.

Expected results:
Both "EXIT trap 1 okay'" and "EXIT trap 2 okay'" should be printed.

Additional info:
As discussed in #480, the ulimit command is an effective way to cause a subshell to fork.

@krader1961
Copy link
Contributor

Note that this change in behavior is present in the ksh93v- release on macOS and thus was not introduced in the past two years as we've worked to modernize the ksh source code to keep it a viable shell. This "bug" is also present using the official ksh93u+ binary (/usr/bin/ksh, /bin/ksh or /bin/ksh93) on OpenSuse 42 but not macOS Catalina or the other Linux VMs I have setup. Which means that either the difference in behavior depends on the options used to build the ksh binary and/or the bug dates back at least seven years and OpenSuse didn't pick up a bug fix that other Linux platforms incorporated. Or the problem is more subtle.

I used scare quotes around "bug" in the previous paragraph because the script, when run using an official ksh93u+ binary, behaves differently depending on the platform. Substituting /bin/true for ulimit -t unlimited causes the OpenSuse 42 ksh93u+ binary to behave as expected by the O.P. Whereas using the ksh93v- binary behaves incorrectly on that platform even with that substitution. Which begs the question why using ulimit fails to behave as expected given the belief that, as discussed in issue #480, that using ulimit is "an effective way to cause a subshell to fork."

This issue is almost certainly due to the attempt by the AST team to avoid forking a subshell if at all possible as discussed in issue #480. In other words, the code sometimes makes the wrong decision. This is another example for why the optimization should be dropped. There simply isn't enough interest or justification for the optimization to make it robust. But I'll be happy to review any fixes that prove me wrong.

@krader1961
Copy link
Contributor

Also, the ksh93v- and 2020.0.0 release works as expected on macOS if the script is

(trap 'echo EXIT trap 1 okay' EXIT)
(/usr/bin/true; trap 'echo EXIT trap 2 okay' EXIT)

So something about how the ulimit builtin is handled is broken with respect to subshells. Which doesn't surprise me in the least.

@krader1961 krader1961 added the bug label Dec 22, 2019
@McDutchie
Copy link
Contributor Author

No, you're not getting it. Invoking /usr/bin/true doesn't cause the subshell itself to fork (i.e.: doesn't cause the subshell itself to become a separate process), it only forks a dummy process. The ulimit builtin does cause the subshell itself to fork, by calling sh_subfork(). See #480 (comment) and the preceding comment.

The issue is that EXIT traps stop working after the subshell itself becomes a separate process. So this is actually a problem with forked subshells.

More proof: redirecting standard output within a command substitution also causes the subshell to fork, and the effects of that are the same.

v=$(trap 'echo EXIT trap 1 okay' EXIT); echo "$v"
v=$(: 1>&1; trap 'echo EXIT trap 2 okay' EXIT); echo "$v"

As with the previous test, "trap 1 okay" is printed, "trap 2 okay" isn't.

@jghub
Copy link

jghub commented Dec 28, 2019

just to confirm: this does not work correctly with the starting point of this project (ksh93v-) either. so this, for a change, is indeed not krader's fault -- except that it should have been obvious to start from the stable ksh93u+ -- (contrary to getting the performance a factor of 2-3 down relative to ksh93v- or u+ (quite a feat!), insisting (and succeeding, so far) to make ksh2020 non-POSIX compliant by changing defined cd behaviour and assorted other tragedies that go on here: "modernize the ksh code to keep it a viable shell" -- yeah... :|).

McDutchie added a commit to modernish/modernish that referenced this issue Jan 11, 2020
This stable release ends the testing stage on the 0.16 branch. From
now on, new 0.16.x versions will only be released to fix serious
bugs, if found. Development towards 0.18.0 is continuing on the Git
master branch.

Below is a summary of the most important changes between
v0.16.1-alpha and v0.16.2. For full details, see the git log
(which has very descriptive commit messages).

CHANGED:

  * 'str ematch' now properly supports full POSIX Extended Regular
    Expressions (EREs) on all shells and operating systems,
    including bounds/interval expressions and POSIX character
    classes. On shells without this feature built in, 'str ematch'
    now uses an awk script that converts a POSIX ERE to a
    traditional awk RE before matching it. Related changes:
    - The WRN_EREBOUNDS warning ID is removed.
    - A new WRN_EREMBYTE warning ID is added. If 'thisshellhas
      WRN_EREMBYTE', you're in a multibyte (UTF-8) locale but the
      system's awk does not support UTF-8 characters, so 'str
      ematch' cannot match these properly in EREs.

  * New 'chdir' command, a robust 'cd' replacement for script use.
    A replacement for 'cd' in scripts is needed because 'cd' has
    features designed for interactive shells that make robust and
    portable use in scripts far too difficult: (1) a user's
    exported $CDPATH could easily cause the wrong directory to be
    entered; (2) certain names are interpreted as special operands
    even after '--', so using arbitrary/untrusted directory names
    is not safe, and (3) the PWD it sets is not safe against
    symlink attacks by default.
       It is possible to work around these problems, but this is
    only really doable with a library function like chdir. The use
    of 'cd' in modernish scripts is now deprecated.

  * Modernish commands that handle long-form shell options (such as
    'push -o OPTION', 'pop -o OPTION') now deal correctly with
    abbreviated shell option names on shells where these can
    natively be abbreviated: ksh93 and yash. This capability is now
    detected under the ID QRK_OPTABBR.

  * mktemp (sys/base/mktemp):
    - the -t option now uses the XDG_RUNTIME_DIR environment
      variable if set. It also no longer dies if that or TMPDIR has
      a bad value; it just won't use it.
    - the -C autocleanup option can now be used in a subshell
      alongside -s and using the tmp file name left in $REPLY
      (not that autocleanup happens on exiting the subshell).

  * readlink (sys/base/readlink):
    - Added the canonicalisation options '-e' (all pathname
      components must exist) and '-m' (none need exist) as in
      GNU 'readlink'.
    - Now supports //UNC/network/paths on Cygwin.

  * which (sys/base/which) now converts relative paths it finds
    to absolute paths, a behaviour that matches GNU 'which'.

  * var/loop: Iteration generator processes can now read from
    standard input as normal (except on interactive shells).
    And stdout is now redirected to stderr instead of closed.

  * LOOP find (var/loop/find):
    - Now supports portable use of the GNU-style '-mindepth n' and
      '-maxdepth n' options, as well as the BSD-style '-depth n',
      '-depth -n', '-depth +n' expression primary. These are
      internally translated to a portable expression.
    - Since loop iteration generators can now read from standard
      input, LOOP find now accepts the -ok and -okdir primaries.

  * In modernish safe pathname expansion (var/local, var/loop),
    resulting paths starting with '+' are now also automatically
    escaped by prefixing './', because some commands have options
    or special operands that start with '+'.

  * New example program: Modernish Dice.
    See: share/doc/modernish/dice.sh

  * Unfortunately, the latest Korn shell 2020.0.0 release is too
    buggy to run modernish. One of its many bugs is that EXIT traps
    don't work in subshells: att/ast#1452
    There is no possible workaround, so this is a fatal bug.
    Modernish now refuses to initialise upon detecting this bug.
    The only ksh93 release known to work is AJM 93u+ 2012-08-01.

  * Changes in shell capability IDs for thisshellhas:
    - OPTNOPREFIX is renamed to QRK_OPTNOPRFX.
    - New shell bugs/quirks detected and identified:
      BUG_CMDSETPP (mksh <= R57)
      BUG_ASGNLOCAL (zsh <= 5.7.1)
      BUG_CASEPAREN (bash 3.2)
    - New shell quirks detected and identified:
      QRK_ANDORBG (zsh)
      QRK_OPTABBR (ksh93, yash)

  * On interactive shells, if a command in a multi-process job
    calls die(), the entire job is now reliably interrupted.

FIXED:

  * On shells with BUG_PUTIOERR, the var/loop module now refuses to
    initialise if SIGPIPE is ignored. This avoids loop iteration
    generators being stuck writing infinitely into the void.

  * thisshellhas: the --cache or --show options could end up in
    an infinite loop upon encountering a cap/*.t file with an
    invalid ID name.

  * shellquote: assignment-arguments didn't work for values
    exempt from quoting like: shellquote var=[.

  * sys/base/seq: the -f option did not correctly handle arbitrary
    format strings.

  * put/putln died on I/O error on FreeBSD sh.

  * str:
    - For an empty variable $empty (which is removed when unquoted,
      even in the safe mode), str returned the wrong result for:
            str {eq,ne,in,begin,end,match} $empty ""
            str isvarname $empty
    - str match: handling of a dangling final backslash was broken.
    - str ematch: on shells where this depends on awk, arguments
      starting with a dash failed because they were misinterpreted
      as awk options.

  * mapr (var/mapr):
    - Empty input caused an infinite loop.
    - "Arguments list too long" errors still occasionally occurred
      on some systems when using mapr with external commands. Fixed
      by using a much more conservative arguments length limit.

  * LOOP select (var/loop/select) didn't cope with REPLY being
    unset within the loop.

  * trace (sys/cmd/harden): Traced commands died on status 128.

  * use: Modules could be erroneously loaded/initialised more than
    once if a slightly different variant of the name was given: a
    leading or trailing slash, or a double slash. Fixed by
    detecting these variants as invalid.

  * % (sys/cmd/procsubst):
    - Would hang on AT&T ksh93. Fixed by forcing the non-forked
      command substitution subshell to fork.
    - Work around an AT&T ksh93 bug with the 'command' command,
      so that e.g. 'cat < $(% command ls)' now works on ksh93.
    - With '% -o', standard output was closed for the launched
      command. Fixed by redirecting it to standard error.
    - Die on no command given and on empty command.

  * LOOP repeat (var/loop/repeat): died with a spurious "invalid
    arithmetic expression" error message on 0 interations.

  * readlink (sys/base/readlink):
    - The --help option wasn't parsed correctly.
    - When canonicalising a path ending in '/', readlink would
      erroneously print the final '/'.
    - Encountering a recursive symlink would cause an infinite
      loop. Fixed by keeping track of symlinks already seen.

  * which (sys/base/which): Given an existing '/command' in the
    root directory, 'PATH=/:$PATH; which command' yielded
    '//command', and 'which /command' did not work at all.

  * var/stack/trap:
    - The logic for resending a signal (in case traps were pushed
      but no POSIX trap is set) was broken, so that e.g. TSTP
      failed to stop the shell, among other bugs.
    - On some shells, CHLD stack traps would trigger themselves,
      causing infinite signal loops.
    - DIE/INT traps on interactive shells were broken when they
      themselves called die(), causing ugly misbehaviour.
    - On zsh, 'trap' did not print ZERR/ERR traps correctly.
    - poptrap: A stack corruption fatal error occurred when
      attempting to pop a trap in a subshell.

OPTIMISED:

  * var/loop/select: Stop iteration generator while waiting for
    reply. This makes 'LOOP select' much better behaved: we avoid
    filling up the interprocess buffer while still remaining
    responsive if a user gives many replies in quick succession.
McDutchie added a commit to modernish/modernish that referenced this issue Jan 11, 2020
This stable release ends the testing stage on the 0.16 branch. From
now on, new 0.16.x versions will only be released to fix serious
bugs, if found. Development towards 0.18.0 is continuing on the Git
master branch.

Below is a summary of the most important changes between
v0.16.1-alpha and v0.16.2. For full details, see the git log
(which has very descriptive commit messages).

CHANGED:

  * 'str ematch' now properly supports full POSIX Extended Regular
    Expressions (EREs) on all shells and operating systems,
    including bounds/interval expressions and POSIX character
    classes. On shells without this feature built in, 'str ematch'
    now uses an awk script that converts a POSIX ERE to a
    traditional awk RE before matching it. Related changes:
    - The WRN_EREBOUNDS warning ID is removed.
    - A new WRN_EREMBYTE warning ID is added. If 'thisshellhas
      WRN_EREMBYTE', you're in a multibyte (UTF-8) locale but the
      system's awk does not support UTF-8 characters, so 'str
      ematch' cannot match these properly in EREs.

  * New 'chdir' command, a robust 'cd' replacement for script use.
    A replacement for 'cd' in scripts is needed because 'cd' has
    features designed for interactive shells that make robust and
    portable use in scripts far too difficult: (1) a user's
    exported $CDPATH could easily cause the wrong directory to be
    entered; (2) certain names are interpreted as special operands
    even after '--', so using arbitrary/untrusted directory names
    is not safe, and (3) the PWD it sets is not safe against
    symlink attacks by default.
       It is possible to work around these problems, but this is
    only really doable with a library function like chdir. The use
    of 'cd' in modernish scripts is now deprecated.

  * Modernish commands that handle long-form shell options (such as
    'push -o OPTION', 'pop -o OPTION') now deal correctly with
    abbreviated shell option names on shells where these can
    natively be abbreviated: ksh93 and yash. This capability is now
    detected under the ID QRK_OPTABBR.

  * mktemp (sys/base/mktemp):
    - the -t option now uses the XDG_RUNTIME_DIR environment
      variable if set. It also no longer dies if that or TMPDIR has
      a bad value; it just won't use it.
    - the -C autocleanup option can now be used in a subshell
      alongside -s and using the tmp file name left in $REPLY
      (not that autocleanup happens on exiting the subshell).

  * readlink (sys/base/readlink):
    - Added the canonicalisation options '-e' (all pathname
      components must exist) and '-m' (none need exist) as in
      GNU 'readlink'.
    - Now supports //UNC/network/paths on Cygwin.

  * which (sys/base/which) now converts relative paths it finds
    to absolute paths, a behaviour that matches GNU 'which'.

  * var/loop: Iteration generator processes can now read from
    standard input as normal (except on interactive shells).
    And stdout is now redirected to stderr instead of closed.

  * LOOP find (var/loop/find):
    - Now supports portable use of the GNU-style '-mindepth n' and
      '-maxdepth n' options, as well as the BSD-style '-depth n',
      '-depth -n', '-depth +n' expression primary. These are
      internally translated to a portable expression.
    - Since loop iteration generators can now read from standard
      input, LOOP find now accepts the -ok and -okdir primaries.

  * In modernish safe pathname expansion (var/local, var/loop),
    resulting paths starting with '+' are now also automatically
    escaped by prefixing './', because some commands have options
    or special operands that start with '+'.

  * New example program: Modernish Dice.
    See: share/doc/modernish/dice.sh

  * Unfortunately, the latest Korn shell 2020.0.0 release is too
    buggy to run modernish. One of its many bugs is that EXIT traps
    don't work in subshells: att/ast#1452
    There is no possible workaround, so this is a fatal bug.
    Modernish now refuses to initialise upon detecting this bug.
    The only ksh93 release known to work is AJM 93u+ 2012-08-01.

  * Changes in shell capability IDs for thisshellhas:
    - OPTNOPREFIX is renamed to QRK_OPTNOPRFX.
    - New shell bugs/quirks detected and identified:
      BUG_CMDSETPP (mksh <= R57)
      BUG_ASGNLOCAL (zsh <= 5.7.1)
      BUG_CASEPAREN (bash 3.2)
    - New shell quirks detected and identified:
      QRK_ANDORBG (zsh)
      QRK_OPTABBR (ksh93, yash)

  * On interactive shells, if a command in a multi-process job
    calls die(), the entire job is now reliably interrupted.

FIXED:

  * On shells with BUG_PUTIOERR, the var/loop module now refuses to
    initialise if SIGPIPE is ignored. This avoids loop iteration
    generators being stuck writing infinitely into the void.

  * thisshellhas: the --cache or --show options could end up in
    an infinite loop upon encountering a cap/*.t file with an
    invalid ID name.

  * shellquote: assignment-arguments didn't work for values
    exempt from quoting like: shellquote var=[.

  * sys/base/seq: the -f option did not correctly handle arbitrary
    format strings.

  * put/putln died on I/O error on FreeBSD sh.

  * str:
    - For an empty variable $empty (which is removed when unquoted,
      even in the safe mode), str returned the wrong result for:
            str {eq,ne,in,begin,end,match} $empty ""
            str isvarname $empty
    - str match: handling of a dangling final backslash was broken.
    - str ematch: on shells where this depends on awk, arguments
      starting with a dash failed because they were misinterpreted
      as awk options.

  * mapr (var/mapr):
    - Empty input caused an infinite loop.
    - "Arguments list too long" errors still occasionally occurred
      on some systems when using mapr with external commands. Fixed
      by using a much more conservative arguments length limit.

  * LOOP select (var/loop/select) didn't cope with REPLY being
    unset within the loop.

  * trace (sys/cmd/harden): Traced commands died on status 128.

  * use: Modules could be erroneously loaded/initialised more than
    once if a slightly different variant of the name was given: a
    leading or trailing slash, or a double slash. Fixed by
    detecting these variants as invalid.

  * % (sys/cmd/procsubst):
    - Would hang on AT&T ksh93. Fixed by forcing the non-forked
      command substitution subshell to fork.
    - Work around an AT&T ksh93 bug with the 'command' command,
      so that e.g. 'cat < $(% command ls)' now works on ksh93.
    - With '% -o', standard output was closed for the launched
      command. Fixed by redirecting it to standard error.
    - Die on no command given and on empty command.

  * LOOP repeat (var/loop/repeat): died with a spurious "invalid
    arithmetic expression" error message on 0 interations.

  * readlink (sys/base/readlink):
    - The --help option wasn't parsed correctly.
    - When canonicalising a path ending in '/', readlink would
      erroneously print the final '/'.
    - Encountering a recursive symlink would cause an infinite
      loop. Fixed by keeping track of symlinks already seen.

  * which (sys/base/which): Given an existing '/command' in the
    root directory, 'PATH=/:$PATH; which command' yielded
    '//command', and 'which /command' did not work at all.

  * var/stack/trap:
    - The logic for resending a signal (in case traps were pushed
      but no POSIX trap is set) was broken, so that e.g. TSTP
      failed to stop the shell, among other bugs.
    - On some shells, CHLD stack traps would trigger themselves,
      causing infinite signal loops.
    - DIE/INT traps on interactive shells were broken when they
      themselves called die(), causing ugly misbehaviour.
    - On zsh, 'trap' did not print ZERR/ERR traps correctly.
    - poptrap: A stack corruption fatal error occurred when
      attempting to pop a trap in a subshell.

OPTIMISED:

  * var/loop/select: Stop iteration generator while waiting for
    reply. This makes 'LOOP select' much better behaved: we avoid
    filling up the interprocess buffer while still remaining
    responsive if a user gives many replies in quick succession.
McDutchie added a commit to modernish/modernish that referenced this issue Jan 13, 2020
Version 0.16.2 quietly failed to initialise on the one supported
ksh93 version (93u+ 2012-08-01). This version fixes that. There are
no other changes. 0.16.2 is withdrawn and replaced by 0.16.3.

Below are the release notes for 0.16.2, which fully apply.
____________________________________
v0.16.2: first stable 0.16.x release

This stable release ends the testing stage on the 0.16 branch. From
now on, new 0.16.x versions will only be released to fix serious
bugs, if found. Development towards 0.18.0 is continuing on the Git
master branch.

Below is a summary of the most important changes between
v0.16.1-alpha and v0.16.2. For full details, see the git log
(which has very descriptive commit messages).

CHANGED:

  * 'str ematch' now properly supports full POSIX Extended Regular
    Expressions (EREs) on all shells and operating systems,
    including bounds/interval expressions and POSIX character
    classes. On shells without this feature built in, 'str ematch'
    now uses an awk script that converts a POSIX ERE to a
    traditional awk RE before matching it. Related changes:
    - The WRN_EREBOUNDS warning ID is removed.
    - A new WRN_EREMBYTE warning ID is added. If 'thisshellhas
      WRN_EREMBYTE', you're in a multibyte (UTF-8) locale but the
      system's awk does not support UTF-8 characters, so 'str
      ematch' cannot match these properly in EREs.

  * New 'chdir' command, a robust 'cd' replacement for script use.
    A replacement for 'cd' in scripts is needed because 'cd' has
    features designed for interactive shells that make robust and
    portable use in scripts far too difficult: (1) a user's
    exported $CDPATH could easily cause the wrong directory to be
    entered; (2) certain names are interpreted as special operands
    even after '--', so using arbitrary/untrusted directory names
    is not safe, and (3) the PWD it sets is not safe against
    symlink attacks by default.
       It is possible to work around these problems, but this is
    only really doable with a library function like chdir. The use
    of 'cd' in modernish scripts is now deprecated.

  * Modernish commands that handle long-form shell options (such as
    'push -o OPTION', 'pop -o OPTION') now deal correctly with
    abbreviated shell option names on shells where these can
    natively be abbreviated: ksh93 and yash. This capability is now
    detected under the ID QRK_OPTABBR.

  * mktemp (sys/base/mktemp):
    - the -t option now uses the XDG_RUNTIME_DIR environment
      variable if set. It also no longer dies if that or TMPDIR has
      a bad value; it just won't use it.
    - the -C autocleanup option can now be used in a subshell
      alongside -s and using the tmp file name left in $REPLY
      (not that autocleanup happens on exiting the subshell).

  * readlink (sys/base/readlink):
    - Added the canonicalisation options '-e' (all pathname
      components must exist) and '-m' (none need exist) as in
      GNU 'readlink'.
    - Now supports //UNC/network/paths on Cygwin.

  * which (sys/base/which) now converts relative paths it finds
    to absolute paths, a behaviour that matches GNU 'which'.

  * var/loop: Iteration generator processes can now read from
    standard input as normal (except on interactive shells).
    And stdout is now redirected to stderr instead of closed.

  * LOOP find (var/loop/find):
    - Now supports portable use of the GNU-style '-mindepth n' and
      '-maxdepth n' options, as well as the BSD-style '-depth n',
      '-depth -n', '-depth +n' expression primary. These are
      internally translated to a portable expression.
    - Since loop iteration generators can now read from standard
      input, LOOP find now accepts the -ok and -okdir primaries.

  * In modernish safe pathname expansion (var/local, var/loop),
    resulting paths starting with '+' are now also automatically
    escaped by prefixing './', because some commands have options
    or special operands that start with '+'.

  * New example program: Modernish Dice.
    See: share/doc/modernish/dice.sh

  * Unfortunately, the latest Korn shell 2020.0.0 release is too
    buggy to run modernish. One of its many bugs is that EXIT traps
    don't work in subshells: att/ast#1452
    There is no possible workaround, so this is a fatal bug.
    Modernish now refuses to initialise upon detecting this bug.
    The only ksh93 release known to work is AJM 93u+ 2012-08-01.

  * Changes in shell capability IDs for thisshellhas:
    - OPTNOPREFIX is renamed to QRK_OPTNOPRFX.
    - New shell bugs/quirks detected and identified:
      BUG_CMDSETPP (mksh <= R57)
      BUG_ASGNLOCAL (zsh <= 5.7.1)
      BUG_CASEPAREN (bash 3.2)
    - New shell quirks detected and identified:
      QRK_ANDORBG (zsh)
      QRK_OPTABBR (ksh93, yash)

  * On interactive shells, if a command in a multi-process job
    calls die(), the entire job is now reliably interrupted.

FIXED:

  * On shells with BUG_PUTIOERR, the var/loop module now refuses to
    initialise if SIGPIPE is ignored. This avoids loop iteration
    generators being stuck writing infinitely into the void.

  * thisshellhas: the --cache or --show options could end up in
    an infinite loop upon encountering a cap/*.t file with an
    invalid ID name.

  * shellquote: assignment-arguments didn't work for values
    exempt from quoting like: shellquote var=[.

  * sys/base/seq: the -f option did not correctly handle arbitrary
    format strings.

  * put/putln died on I/O error on FreeBSD sh.

  * str:
    - For an empty variable $empty (which is removed when unquoted,
      even in the safe mode), str returned the wrong result for:
            str {eq,ne,in,begin,end,match} $empty ""
            str isvarname $empty
    - str match: handling of a dangling final backslash was broken.
    - str ematch: on shells where this depends on awk, arguments
      starting with a dash failed because they were misinterpreted
      as awk options.

  * mapr (var/mapr):
    - Empty input caused an infinite loop.
    - "Arguments list too long" errors still occasionally occurred
      on some systems when using mapr with external commands. Fixed
      by using a much more conservative arguments length limit.

  * LOOP select (var/loop/select) didn't cope with REPLY being
    unset within the loop.

  * trace (sys/cmd/harden): Traced commands died on status 128.

  * use: Modules could be erroneously loaded/initialised more than
    once if a slightly different variant of the name was given: a
    leading or trailing slash, or a double slash. Fixed by
    detecting these variants as invalid.

  * % (sys/cmd/procsubst):
    - Would hang on AT&T ksh93. Fixed by forcing the non-forked
      command substitution subshell to fork.
    - Work around an AT&T ksh93 bug with the 'command' command,
      so that e.g. 'cat < $(% command ls)' now works on ksh93.
    - With '% -o', standard output was closed for the launched
      command. Fixed by redirecting it to standard error.
    - Die on no command given and on empty command.

  * LOOP repeat (var/loop/repeat): died with a spurious "invalid
    arithmetic expression" error message on 0 interations.

  * readlink (sys/base/readlink):
    - The --help option wasn't parsed correctly.
    - When canonicalising a path ending in '/', readlink would
      erroneously print the final '/'.
    - Encountering a recursive symlink would cause an infinite
      loop. Fixed by keeping track of symlinks already seen.

  * which (sys/base/which): Given an existing '/command' in the
    root directory, 'PATH=/:$PATH; which command' yielded
    '//command', and 'which /command' did not work at all.

  * var/stack/trap:
    - The logic for resending a signal (in case traps were pushed
      but no POSIX trap is set) was broken, so that e.g. TSTP
      failed to stop the shell, among other bugs.
    - On some shells, CHLD stack traps would trigger themselves,
      causing infinite signal loops.
    - DIE/INT traps on interactive shells were broken when they
      themselves called die(), causing ugly misbehaviour.
    - On zsh, 'trap' did not print ZERR/ERR traps correctly.
    - poptrap: A stack corruption fatal error occurred when
      attempting to pop a trap in a subshell.

OPTIMISED:

  * var/loop/select: Stop iteration generator while waiting for
    reply. This makes 'LOOP select' much better behaved: we avoid
    filling up the interprocess buffer while still remaining
    responsive if a user gives many replies in quick succession.
@jghub
Copy link

jghub commented Jan 27, 2020

although implied in the original post, in case this is not completely clear: this is a bug in ksh93v- and in ksh2020. but it works correctly in ksh93u+ (tested with, both, OSX and ubuntu) -- like many other things that are buggy in ksh2020 but work just fine in ksh93u+

JohnoKing added a commit to JohnoKing/ksh that referenced this issue Mar 12, 2021
src/cmd/ksh93/tests/arrays.sh,
src/cmd/ksh93/tests/arrays2.sh:
- Backport some regression tests from ksh93v- for associative arrays.

src/cmd/ksh93/tests/basic.sh:
- Add ksh93v- regression tests for background process output in
  backtick and shared-state command substitutions as well as functions
  used in command substitutions.
- Add a regression tests for using EXIT traps in subshells. In ksh93v-
  and ksh2020 EXIT traps don't work in forked subshells:
  att#1452
- The trap builtin shouldn't segfault after receiving an invalid
  signal name. ksh2020 regression: att#1403
- Add a test to make sure invalid flags don't crash ksh.
  ksh2020 regression: att#1284
- Test for an illegal seek error when using the 'join' command with
  process substitutions. ksh93v- regression:
  https://www.mail-archive.com/ast-users@lists.research.att.com/msg00816.html

src/cmd/ksh93/tests/bracket.sh:
- Add some regression tests from ksh93v- for the -eq test operator.

src/cmd/ksh93/tests/builtins.sh:
- Move the regression test for 'exit' in an interactive shell to
  the exit.sh script.
- Test for assignments preceding the command builtin persisting
  after an error. ksh2020 regression: att#1402
- The chmod builtin should modify the permissions of all files passed
  to it. ksh2020 regression: att#949
- Add regression tests for the cd builtin. In ksh93v- 2013-10-10 alpha,
  using cd on a directory without an execute bit doesn't fail.
  The test for using cd on a normal file was backported from
  ksh93v-.
- Backport a ksh93v- regression test for the exit status
  from 'kill %'.

src/cmd/ksh93/tests/case.sh:
- Test for a segfault when ksh handles an invalid character
  class in a pattern. ksh2020 regression: att#1409

src/cmd/ksh93/tests/exit.sh:
- Add regression tests from ksh2020 for the 'exit' builtin:
  att@d9491d4

src/cmd/ksh93/tests/io.sh:
- Add a regression test from ksh93v- for a process substitution
  hang. This test fails in the 93v- 2013 alpha but succeeds in
  the 2014 beta.

src/cmd/ksh93/tests/math.sh:
- 'typeset -s foo=30000' adds garbage to $foo in ksh93u+, ksh93v-
  and ksh2020:
  $ typeset -s foo=30000
  $ echo $foo
  5#1430000
  This bug was fixed in commit 88a6baa, but that commit didn't
  add a regression test for it.

src/cmd/ksh93/tests/variables.sh:
- Add a regression test for $PS4 incorrectly unsetting ${.sh.subshell}:
  att#1092
JohnoKing added a commit to JohnoKing/ksh that referenced this issue Mar 12, 2021
src/cmd/ksh93/tests/arrays.sh,
src/cmd/ksh93/tests/arrays2.sh:
- Backport some regression tests from ksh93v- for associative arrays.

src/cmd/ksh93/tests/basic.sh:
- Add ksh93v- regression tests for background process output in
  backtick and shared-state command substitutions as well as functions
  used in command substitutions.
- Add regression tests for using EXIT traps in subshells. In ksh93v-
  and ksh2020 EXIT traps don't work in forked subshells:
  att#1452
- The trap builtin shouldn't segfault after receiving an invalid
  signal name. ksh2020 regression: att#1403
- Add a test to make sure invalid flags don't crash ksh.
  ksh2020 regression: att#1284
- Test for an illegal seek error when using the 'join' command with
  process substitutions. ksh93v- regression:
  https://www.mail-archive.com/ast-users@lists.research.att.com/msg00816.html

src/cmd/ksh93/tests/bracket.sh:
- Add some regression tests from ksh93v- for the -eq test operator.

src/cmd/ksh93/tests/builtins.sh:
- Move the regression test for 'exit' in an interactive shell to
  the exit.sh script.
- Test for assignments preceding the command builtin persisting
  after an error. ksh2020 regression: att#1402
- The chmod builtin should modify the permissions of all files passed
  to it. ksh2020 regression: att#949
- Add regression tests for the cd builtin. In ksh93v- 2013-10-10 alpha,
  using cd on a directory without an execute bit doesn't cause an
  error. The test for using cd on a normal file was backported
  from ksh93v-.
- Backport a ksh93v- regression test for the exit status
  from 'kill %'.

src/cmd/ksh93/tests/case.sh:
- Test for a segfault when ksh handles an invalid character
  class in a pattern. ksh2020 regression: att#1409

src/cmd/ksh93/tests/exit.sh:
- Add regression tests from ksh2020 for the 'exit' builtin:
  att@d9491d4

src/cmd/ksh93/tests/io.sh:
- Add a regression test from ksh93v- for a process substitution
  hang. This test fails in the 93v- 2013 alpha but succeeds in
  the 2014 beta.

src/cmd/ksh93/tests/math.sh:
- 'typeset -s foo=30000' adds garbage to $foo in ksh93u+, ksh93v-
  and ksh2020:
  $ typeset -s foo=30000
  $ echo $foo
  5#1430000
  This bug was fixed in commit 88a6baa, but that commit didn't
  add a regression test for it.

src/cmd/ksh93/tests/variables.sh:
- Add a regression test for $PS4 incorrectly unsetting ${.sh.subshell}:
  att#1092
McDutchie pushed a commit to ksh93/ksh that referenced this issue Mar 12, 2021
src/cmd/ksh93/tests/arrays.sh,
src/cmd/ksh93/tests/arrays2.sh:
- Backport some regression tests from ksh93v- for associative
  arrays.

src/cmd/ksh93/tests/basic.sh:
- Add ksh93v- regression tests for background process output in
  backtick and shared-state command substitutions as well as
  functions used in command substitutions.

- Add regression tests for using EXIT traps in subshells. In
  ksh93v- and ksh2020 EXIT traps don't work in forked subshells:
  att#1452
- The trap builtin shouldn't segfault after receiving an invalid
  signal name. ksh2020 regression:
  att#1403
- Add a test to make sure invalid flags don't crash ksh.
  ksh2020 regression: att#1284
- Test for an illegal seek error when using the 'join' command with
  process substitutions. ksh93v- regression:
  https://www.mail-archive.com/ast-users@lists.research.att.com/msg00816.html

src/cmd/ksh93/tests/bracket.sh:
- Add some regression tests from ksh93v- for the -eq test operator.

src/cmd/ksh93/tests/builtins.sh:
- Move the regression test for 'exit' in an interactive shell to
  the exit.sh script.
- Test for assignments preceding the command builtin persisting
  after an error. ksh2020 regression:
  att#1402
- The chmod builtin should modify the permissions of all files
  passed to it. ksh2020 regression:
  att#949
- Add regression tests for the cd builtin. In ksh93v- 2013-10-10
  alpha, using cd on a directory without an execute bit doesn't
  cause an error. The test for using cd on a normal file was
  backported from ksh93v-.
- Backport a ksh93v- regression test for the exit status
  from 'kill %'.

src/cmd/ksh93/tests/case.sh:
- Test for a segfault when ksh handles an invalid character class
  in a pattern. ksh2020 regression:
  att#1409

src/cmd/ksh93/tests/exit.sh:
- Add regression tests from ksh2020 for the 'exit' builtin:
  att@d9491d46

src/cmd/ksh93/tests/io.sh:
- Add a regression test from ksh93v- for a process substitution
  hang. This test fails in the 93v- 2013 alpha but succeeds in
  the 2014 beta.

src/cmd/ksh93/tests/math.sh:
- 'typeset -s foo=30000' adds garbage to $foo in ksh93u+, ksh93v-
  and ksh2020:
  $ typeset -s foo=30000
  $ echo $foo
  5#1430000
  This bug was fixed in commit 88a6baa, but that commit didn't
  add a regression test for it.

src/cmd/ksh93/tests/variables.sh:
- Add a regression test for $PS4 incorrectly unsetting
  ${.sh.subshell}: att#1092
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants