Project

General

Profile

Bug #13512

ksh's >; is not reliable with `-c'

Added by Andy Fiddaman about 2 months ago. Updated about 1 month ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
-
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Bite-size
Tags:
Gerrit CR:

Description

Examples from Josh Clulow:

This does not work and leaves a temporary file but no foo

% ksh93 -c '{ echo "Foo"; echo "bar"; uname; } >; foo'
% ls -a
./                 ../                .<#d_20278{;.tmp

This works:

% ksh93 -c '{ echo "Foo"; echo "bar"; uname; true; } >; foo'
% ls -a
./    ../   foo


Related issues

Related to illumos gate - Bug #13513: Do not use >; in cmd/ast/tools/MakefileClosedAndy Fiddaman

Actions
#1

Updated by Electric Monk about 2 months ago

  • Gerrit CR set to 1225
#2

Updated by Andy Fiddaman about 2 months ago

  • Related to Bug #13513: Do not use >; in cmd/ast/tools/Makefile added
#3

Updated by Andy Fiddaman about 2 months ago

  • Description updated (diff)
#4

Updated by Andy Fiddaman about 2 months ago

  • Gerrit CR deleted (1225)
#6

Updated by Andy Fiddaman about 2 months ago

  • Subject changed from ksh's >; is not reliable to ksh's >; is not reliable with `-c'
#7

Updated by Electric Monk about 2 months ago

  • Gerrit CR set to 1226
#8

Updated by Andy Fiddaman about 2 months ago

Before the change in the associated Gerrit review:

test io begins at 2021-02-12+08:12:27
        io.sh[506]: <>; does not truncate files - xxx - xxx bar
        io.sh[510]: >; does not work with -c
test io failed at 2021-02-12+08:12:39 with exit code 1

and after

test io begins at 2021-02-12+08:08:04
test io passed at 2021-02-12+08:08:17 [ 95 tests 0 errors ]
#9

Updated by Andy Fiddaman about 1 month ago

This is something of a big hammer to disable a buggy feature of ksh93 when
running something via the shell's -c option.

ksh93 has a novel implementation of POSIX subshells, in that it includes a
non-forking implementation. POSIX says of subshells:

A subshell environment shall be created as a duplicate of the shell
environment, except that signal traps that are not being ignored shall be set
to the default action. Changes made to the subshell environment shall not
affect the shell environment. Command substitution, commands that are grouped
with parentheses, and asynchronous lists shall be executed in a subshell
environment. Additionally, each command of a multi-command pipeline is in a
subshell environment; as an extension, however, any or all commands in a
pipeline may be executed in the current environment.

All shells apart from ksh fork off a new process to create a subshell
environment. As far as the history notes say, ksh's non-forking implementation
was in order to support systems which do not have fork(), and as a
performance optimisation since fork() used to be relatively expensive.

However, there a bugs in the non-forking subshell implementation, particularly
around I/O and redirection. From what I read in the issues against
https://github.com/att/ast/issues, every time somebody tries to fix one of
these issues they introduce a regression elsewhere. Often the bugs can be
worked around by deliberately including something to force ksh to use
a forked subshell - I've seen people do things like 1>&1; just to trick
the shell into doing this.

Given that the two main reasons for having non-forking subshells are no longer
relevant for illumos, and they are known to be buggy, this change just disables
their use entirely for ksh -c, resulting in consistent - and correct -
behaviour.

The example in the first post of this issue is not actually a subshell -
{ ;} syntax is just a list of commands which should be executed in the same
shell environment. However, since ksh93 sets the non-forking flag for all -c
invocations, the behaviour changes. For the last command in the list, the shell
chooses to just exec it without forking. When redirection is in progress,
particularly one like >; which requires the shell to do something after
the list is complete, this is just wrong, and this explains why the temporary
file gets left behind.

Here's the example where it decides the last command does not need to fork.

bloody% truss -flea -t fork ksh93 -c '{ echo "Foo"; echo "bar"; uname; } >; foo' 2>&1 | egrep 'fork|exec'
7536/1:         execve("/usr/bin/ksh93", 0x0803DC80, 0x0803DC90)  argc = 3
7536/1:         execve("/usr/bin/amd64/ksh93", 0x0803DC80, 0x0803DC90)  argc = 3
7536/1:         execve("/usr/bin/uname", 0x009F1E50, 0x009F2078)  argc = 1

and adding a built-in after the last command changes that behaviour.

bloody% truss -flea -t fork ksh93 -c '{ echo "Foo"; echo "bar"; uname; true; } >; foo' 2>&1 | egrep 'fork|exec'
7531/1:         execve("/usr/bin/ksh93", 0x0803DB38, 0x0803DB48)  argc = 3
7531/1:         execve("/usr/bin/amd64/ksh93", 0x0803DB38, 0x0803DB48)  argc = 3
7531/1:         vforkx(0)                                       = 7532
7532/1:         vforkx()        (returning as child ...)        = 7531
7532/1:         execve("/usr/bin/uname", 0x00AB9E50, 0x00ABA148)  argc = 1

So, in summary, this change disables the non-forking subshell optimisation
for ksh -c entirely to avoid problems like this at the cost of an extra
fork() occasionally.

#10

Updated by Electric Monk about 1 month ago

  • Status changed from In Progress to Closed
  • % Done changed from 0 to 100

git commit 718fc4ac1dfcd860e967fb444c831507f97cd300

commit  718fc4ac1dfcd860e967fb444c831507f97cd300
Author: Andy Fiddaman <omnios@citrus-it.co.uk>
Date:   2021-02-28T15:19:50.000Z

    13512 ksh's >; is not reliable with `-c'
    Reviewed by: Toomas Soome <tsoome@me.com>
    Reviewed by: Gordon Ross <gordon.w.ross@gmail.com>
    Reviewed by: Rich Lowe <richlowe@richlowe.net>
    Approved by: Dan McDonald <danmcd@joyent.com>

Also available in: Atom PDF