NetBSD Problem Report #54743

From www@netbsd.org  Sat Dec  7 05:44:46 2019
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 2D46B7A1A2
	for <gnats-bugs@gnats.NetBSD.org>; Sat,  7 Dec 2019 05:44:46 +0000 (UTC)
Message-Id: <20191207054445.4F2DC7A1C1@mollari.NetBSD.org>
Date: Sat,  7 Dec 2019 05:44:45 +0000 (UTC)
From: iku.iwasa@gmail.com
Reply-To: iku.iwasa@gmail.com
To: gnats-bugs@NetBSD.org
Subject: sh(1) exits with code 127 by wait after trap
X-Send-Pr-Version: www-1.0

>Number:         54743
>Category:       bin
>Synopsis:       sh(1) exits with code 127 by wait after trap
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kre
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat Dec 07 05:45:00 +0000 2019
>Closed-Date:    Sat Dec 28 13:49:46 +0000 2019
>Last-Modified:  Sat Dec 28 13:51:50 +0000 2019
>Originator:     Iku Iwasa
>Release:        NetBSD 9.0 RC1
>Organization:
>Environment:
>Description:
With sh(1) on NetBSD 9.0 RC1, the following command exits with code 127.

```
$ sh -c 'sleep 604800 & sleep=$!; trap "exit 0" 15; wait $sleep'
```

It worked on NetBSD 8.1 and works with ksh(1), bash, zsh.

The above command is simplified version of what "with-editor" uses for remote host.
"with-editor" is an Emacs package that Magit (Emacs git interface https://magit.vc/) uses.
As a result, Magit does not work with remote NetBSD host unless the configuration is changed to use e.g. ksh(1).
>How-To-Repeat:
Run

$ sh -c 'sleep 604800 & sleep=$!; trap "exit 0" 15; wait $sleep'
>Fix:

>Release-Note:

>Audit-Trail:

Responsible-Changed-From-To: bin-bug-people->kre
Responsible-Changed-By: kre@NetBSD.org
Responsible-Changed-When: Sat, 07 Dec 2019 06:52:13 +0000
Responsible-Changed-Why:
I am looking into this PR


State-Changed-From-To: open->analyzed
State-Changed-By: kre@NetBSD.org
State-Changed-When: Sat, 07 Dec 2019 09:36:57 +0000
State-Changed-Why:
I know what is happening, and have a fix.   It needs a bit more
testing before it gets committed.   It will need a pullup to -9.


From: iquiw <iku.iwasa@gmail.com>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: bin/54743 (sh(1) exits with code 127 by wait after trap)
Date: Sun, 8 Dec 2019 11:08:02 +0900

 Sorry, I forgot to mention about "with-editor"'s actual usage and the
 expected result.

 "with-editor"'s actual command is the following.
 https://github.com/magit/with-editor/blob/v2.8.3/with-editor.el#L174-L180
 ```
 $ sh -c 'echo "WITH-EDITOR: $$ OPEN $0  IN $(pwd)"; sleep 604800 &
 sleep=$!; trap "kill $sleep; exit 0" USR1; trap "kill $sleep; exit 1"
 USR2; wait $sleep'
 ```
 And it expects exit code provided by "exit" in "trap".
 That is, exit code 0 if "sleep" is killed by "USR1" signal. And exit
 code 1 if "sleep" is killed by "USR2" signal.

 For the simplified version in the original report, exit code 0 is
 expected when "sleep" is killed by "TERM" signal.

From: iquiw <iku.iwasa@gmail.com>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: bin/54743 (sh(1) exits with code 127 by wait after trap)
Date: Sun, 8 Dec 2019 11:14:06 +0900

 > And it expects exit code provided by "exit" in "trap".
 > That is, exit code 0 if "sleep" is killed by "USR1" signal. And exit
 > code 1 if "sleep" is killed by "USR2" signal.

 Ah, the above is wrong. Not "sleep", but "sh" is killed.

From: Robert Elz <kre@munnari.OZ.AU>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: bin/54743 (sh(1) exits with code 127 by wait after trap)
Date: Sun, 08 Dec 2019 11:49:16 +0700

     Date:        Sun,  8 Dec 2019 02:10:01 +0000 (UTC)
     From:        iquiw <iku.iwasa@gmail.com>
     Message-ID:  <20191208021001.97FD87A18F@mollari.NetBSD.org>

   |  And it expects exit code provided by "exit" in "trap".
   |  That is, exit code 0 if "sleep" is killed by "USR1" signal. And exit
   |  code 1 if "sleep" is killed by "USR2" signal.

 Actually it isn't the sleep that receives one of those signals
 (which would be difficult to arrange, as the outside world doesn't
 know the pid of that sleep process) but the shell that is running
 that script.

 But that's just a quibble - I know what is meant there, and that
 isn't working properly either.  This one is a new (different) bug...

 I'll fix this one as well.

 We really need some (real) sh tests for the trap builtin which would
 help find some of these issues earlier.   One day...

 kre


From: "Robert Elz" <kre@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/54743 CVS commit: src/bin/sh
Date: Mon, 9 Dec 2019 00:14:24 +0000

 Module Name:	src
 Committed By:	kre
 Date:		Mon Dec  9 00:14:24 UTC 2019

 Modified Files:
 	src/bin/sh: eval.c

 Log Message:
 PR bin/54743

 Having traps set should not enforce a fork for the next command,
 whatever that command happens to be, only for commands which would
 normally fork if they weren't the last command expected to be
 executed (ie: builtins and functions shouldn't be exexuted in a
 sub-shell merely because a trap is set).

 As it was (for example)
 	trap 'whatever' SIGANY; wait $anypid
 was guaranteed to fail the wait, as the subshell it was executed
 in could not have any children.

 XXX pullup -9


 To generate a diff of this commit:
 cvs rdiff -u -r1.175 -r1.176 src/bin/sh/eval.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Robert Elz" <kre@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/54743 CVS commit: src/bin/sh
Date: Mon, 9 Dec 2019 00:14:30 +0000

 Module Name:	src
 Committed By:	kre
 Date:		Mon Dec  9 00:14:30 UTC 2019

 Modified Files:
 	src/bin/sh: trap.c

 Log Message:
 PR bin/54743

 If a builtin command or function is the final command intended to be
 executed, and is interrupted by a caught signal, the trap handler for
 that signal was not executed - the shell simply exited (an exit trap
 handler would still have been run - if there was one the handler
 for the signal may have been invoked during the execution of the
 exit trap handler, which, if it happened, is incorrect sequencing).

 Now, if we're exiting, and there are pending signals, run their handlers
 just before running the EXIT trap handler, if any.

 There are almost certainly plenty more issues with traps that need
 solving.   Later,

 XXX pullup -9

 (-8 is too different in this area, and this problem suitably obscure,
 that we won't bother)     (the -7 sh is simply obsolete).


 To generate a diff of this commit:
 cvs rdiff -u -r1.52 -r1.53 src/bin/sh/trap.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: analyzed->needs-pullups
State-Changed-By: kre@NetBSD.org
State-Changed-When: Mon, 09 Dec 2019 00:59:58 +0000
State-Changed-Why:
The fixes for the issues here need pulling up to -9 
(after settling in HEAD for a day or two)


State-Changed-From-To: needs-pullups->pending-pullups
State-Changed-By: kre@NetBSD.org
State-Changed-When: Wed, 11 Dec 2019 05:46:21 +0000
State-Changed-Why:
[pullup-9 #542] requested


From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/54743 CVS commit: [netbsd-9] src/bin/sh
Date: Wed, 11 Dec 2019 14:52:50 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Wed Dec 11 14:52:50 UTC 2019

 Modified Files:
 	src/bin/sh [netbsd-9]: eval.c trap.c

 Log Message:
 Pull up following revision(s) (requested by kre in ticket #542):

 	bin/sh/eval.c: revision 1.176
 	bin/sh/trap.c: revision 1.53

 PR bin/54743

 Having traps set should not enforce a fork for the next command,
 whatever that command happens to be, only for commands which would
 normally fork if they weren't the last command expected to be
 executed (ie: builtins and functions shouldn't be exexuted in a
 sub-shell merely because a trap is set).

 As it was (for example)
 	trap 'whatever' SIGANY; wait $anypid
 was guaranteed to fail the wait, as the subshell it was executed
 in could not have any children.

 XXX pullup -9

 PR bin/54743

 If a builtin command or function is the final command intended to be
 executed, and is interrupted by a caught signal, the trap handler for
 that signal was not executed - the shell simply exited (an exit trap
 handler would still have been run - if there was one the handler
 for the signal may have been invoked during the execution of the
 exit trap handler, which, if it happened, is incorrect sequencing).

 Now, if we're exiting, and there are pending signals, run their handlers
 just before running the EXIT trap handler, if any.
 There are almost certainly plenty more issues with traps that need
 solving.   Later,

 XXX pullup -9
 (-8 is too different in this area, and this problem suitably obscure,
 that we won't bother)     (the -7 sh is simply obsolete).


 To generate a diff of this commit:
 cvs rdiff -u -r1.175 -r1.175.2.1 src/bin/sh/eval.c
 cvs rdiff -u -r1.52 -r1.52.2.1 src/bin/sh/trap.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: pending-pullups->feedback
State-Changed-By: kre@NetBSD.org
State-Changed-When: Wed, 11 Dec 2019 16:21:20 +0000
State-Changed-Why:
The fixes for this have now been pulled up to NetBSD -9.
Does it all work correctly for you new (the changes installed
were identical to the patches I sent you to test).


From: iquiw <iku.iwasa@gmail.com>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: bin/54743 (sh(1) exits with code 127 by wait after trap)
Date: Sun, 22 Dec 2019 21:05:38 +0900

 I tested the amd64 image at 21st Dec. Confirmed the fix.
 http://nycdn.netbsd.org/pub/NetBSD-daily/netbsd-9/201912212100Z/

State-Changed-From-To: feedback->closed
State-Changed-By: kre@NetBSD.org
State-Changed-When: Sat, 28 Dec 2019 13:49:46 +0000
State-Changed-Why:
Confirmed fixed by submitter (in a message sent to various
addresses, including gnats-admin, but not gnats-bugs).
[And in a copy of that same message sent to gnats-bugs that I
had not previously noticed.]


>Unformatted:

Home	PR Database Search