Re: [PATCH] Fix exit status of signal handlers in shell scripts

[Bruno Haible]

Hi Dmitry,

The 'trap' issue is complicated enough that, before answering your mail,
I'd like to point out my understanding of the issue.

Sources:
  - POSIX 
<http://www.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#trap>
  - Autoconf manual 
<http://www.gnu.org/software/autoconf/manual/html_node/Limitations-of-Builtins.html>

Caveats:
  - Subshells: 'trap "" sig' is inherited, other 'trap's are not.
  - In an "exit" command, there is a portability problem with use of '$?' in a
    trap for condition 0, as explained in the Autoconf manual. The workaround
    is to use the    (exit n); exit n     idiom. (Only needed in shell scripts
    that install traps!)
  - For asynchronous signals, use of '$?' is random.

So the recommended use of trap is:
  - For condition 0, use the    (exit n); exit n     idiom and use
    'status=$?' at the beginning of the trap handler and 'exit $status'
    at the end.
  - For conditions other than 0, don't use $?. But put an 'exit n' statement
    into the trap handler.

Dmitry V. Levin wrote:
> There is a comment about shell signal handlers in gnulib-tool saying that
> "The value of $? is 128 + signal number and is set before the
> trap-registered command is run".  Unfortunately, this comment is wrong,
> and it seems to be a widespread misunderstanding.
> ...
> In case of signal handler, the exit status of the last command run
> before the trap might be 128 + signal number, this usually happens when
> the last command before the trap was a process terminated by signal.  In
> other cases, the value of $? may be arbitrary.  Sometimes it's quite
> hard to guess this value due to race conditions.  Here is an example of
> such race condition where the value of $? takes one of 3 different
> values: 0, 1 and 143:
> 
> $ for i in `seq 0 9`; do sh -c 'trap "exit \$?" TERM; while /bin/true; do 
> /bin/false; done' & pid=$! && sleep 0.01 && kill -TERM -$pid && wait $pid; 
> done
> [1] 5122
> Terminated
> [1]+  Exit 143                sh -c 'trap "exit \$?" TERM; while /bin/true; 
> do /bin/false; done'
> [1] 5142
> Terminated
> [1]+  Exit 143                sh -c 'trap "exit \$?" TERM; while /bin/true; 
> do /bin/false; done'
> [1] 5165
> [1]+  Exit 1                  sh -c 'trap "exit \$?" TERM; while /bin/true; 
> do /bin/false; done'
> [1] 5201
> [1]+  Done                    sh -c 'trap "exit \$?" TERM; while /bin/true; 
> do /bin/false; done'
> [1] 5238
> Terminated
> [1]+  Exit 143                sh -c 'trap "exit \$?" TERM; while /bin/true; 
> do /bin/false; done'
> [1] 5271
> [1]+  Exit 1                  sh -c 'trap "exit \$?" TERM; while /bin/true; 
> do /bin/false; done'
> [1] 5306
> Terminated
> [1]+  Exit 143                sh -c 'trap "exit \$?" TERM; while /bin/true; 
> do /bin/false; done'
> [1] 5341
> Terminated
> [1]+  Exit 143                sh -c 'trap "exit \$?" TERM; while /bin/true; 
> do /bin/false; done'
> [1] 5376
> [1]+  Done                    sh -c 'trap "exit \$?" TERM; while /bin/true; 
> do /bin/false; done'
> [1] 5411
> [1]+  Done                    sh -c 'trap "exit \$?" TERM; while /bin/true; 
> do /bin/false; done'

Indeed, thanks for reminding us that '$?' is random at the beginning of a
trap handler for a condition other than 0. That wrong comment came out of
a discussion ending in 
<http://lists.gnu.org/archive/html/bug-gnulib/2006-01/msg00269.html>.

> Proposed patch is attached.

Sorry, but I don't understand why you chose 143 as exit code, making it look
like the process died from SIGTERM when in fact in may have been SIGINT.
There seem to be two conventions for the exit code: either 128 + signal number,
or 1.

  $ sleep 5
  ^C
  $ echo $?
  130
  $ while /bin/true; do /bin/true; done
  ^C^C
  $ echo $?
  1

gnulib-tool and MODULES.html.sh use exit code 1 here. Why not?

> It doesn't fix multiple trap bugs in the 
> test suite, though:
> $ grep -r '\<trap[[:space:]].*15' tests/
> tests/test-vc-list-files-git.sh:trap '(exit $?); exit $?' 1 2 13 15
> tests/test-vc-list-files-cvs.sh:trap '(exit $?); exit $?' 1 2 13 15

This is the same bug.

> tests/uniwidth/test-uc_width2.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
> tests/test-yesno.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
> tests/test-vprintf-posix.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
> tests/test-fpending.sh:trap 'rm -fr $tmpfile' 1 2 3 15
> tests/test-tsearch.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
> tests/test-xstrtoimax.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
> tests/test-perror.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
> tests/test-fprintf-posix.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
> tests/test-select-out.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
> tests/test-binary-io.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
> tests/test-xstrtol.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
> tests/test-dprintf-posix.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
> tests/test-xstrtoumax.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
> tests/test-printf-posix.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
> tests/test-atexit.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
> tests/test-select-in.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
> tests/test-xprintf-posix.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
> tests/test-closein.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
> tests/test-sigpipe.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
> tests/test-lseek.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
> tests/test-vdprintf-posix.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
> tests/test-vfprintf-posix.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
> tests/test-c-stack2.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
> tests/test-c-stack.sh:trap 'rm -fr $tmpfiles' 1 2 3 15

This is a different bug: A 'trap n' for n != 0 should contain an 'exit'
command. Paul highlighted this bug already in
  <http://lists.gnu.org/archive/html/bug-gnulib/2006-01/msg00264.html>
but I forgot about it. For the tests, I am inclined to provide the exit
code '77' (= SKIP), rather than '1' (= FAIL): If a test is terminated
by Ctrl-C, it has neither passed nor failed.

> tests/test-update-copyright.sh:trap 'rm -f $TMP_BASE*' 0 1 2 3 15

Here we have an additional bug: use of a trap for n = 0 and n != 0 at
the same time. This needs to be split into two 'trap' statements before
it can fulfil the recommendations about '$?'.

I'll propose patches for gnulib-tool and most of the tests, and leave
bootstrap, tests/test-vc-list-files*.sh, tests/test-update-copyright.sh
to Jim.

Bruno