dash.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [BUG] Redirection bug in subshell's EXIT trap
@ 2017-10-19 17:44 Vitaly Sinilin
  2017-10-19 21:57 ` Harald van Dijk
  0 siblings, 1 reply; 2+ messages in thread
From: Vitaly Sinilin @ 2017-10-19 17:44 UTC (permalink / raw)
  To: dash

Hi,

I've recently written a quite unusual script and faced a strange dash
behavior that seems not POSIX-ly correct to me.

Here is the script with a lot of debugging extra output.

(I am sorry about posting a pretty lengthy script, but I feel like a
shorten version will look just insane without the context.)

#!/bin/dash
#
# This helper script takes a terminal from stdin to interact with user
# and prints uuid of the selected entity to stdout that is not
# necessarily connected to the same terminal.
#
# It's meant to be run as part of command substitution, e.g.
#
#     uuid=$(pickthing "$@")
#

die() {
	echo "$@" >&2
	exit 2
}

getch() ( # need a subshell here, so round braces instead of curly ones
	dump_self_fds
	trap 'dump_self_fds; stty -cbreak echo' EXIT
	stty cbreak -echo
	head -c 1
	dump_self_fds
)

dump_self_fds() {
	{ read void; read void; read header pid; } < /proc/self/status
	printf "%d:" $pid
	for fd in 0 1 2; do
		printf " %s" "$(readlink /proc/$pid/fd/$fd)"
	done
	echo
} >&2

test -t 0 || die "Standard input is not a terminal"

exec 3>&1 # 3 will be initial stdout
exec 1>$(tty) # switch stdout to terminal
exec 4<&0 # 4 will be initial stdin (for use in pipelines)
exec 0<&-

# Even when run as sh, bash doesn't interrupt script execution if SIGINT
# is received during execution of a foreground pipeline (dash does). So
# we need to explicitly instruct it to do so.
# https://www.cons.org/cracauer/sigint.html
trap 'trap - INT; kill -INT $$' INT

ask() {
	echo "Take $2? (y/n)"
	while :; do
		#key=$(exec 0<&4; dump_self_fds; getch) # works in dash
		key=$(dump_self_fds; getch <&4) # looks better but
		                                # doesn't work in dash
		case $key in
			[yY]) echo $1 >&3; return 0;;
			[nN]) break;;
		esac
	done
	return 1
}

lsthings() { # just a stub instead of a real command
	cat <<-"EOF"
	df66e01c-1dcf-487c-aedc-a8b1c1859b49 item a
	fff43fed-7560-48f7-9b38-c5509d74693f item b
	EOF
}

dump_self_fds
lsthings "$@" | { while read uuid info; do
	dump_self_fds
	ask "$uuid" "$info" && exit 0
done; exit 1; } || ask "$*" "$* (initial literal value)"

# End of script

N.B. dump_self_fds() dumps PID and file descriptors 0, 1 and 2. It is very
Linux-specific. So there is no point in trying this script in other
environments.

N.B. This script will mess up your terminal settings. Use 'reset' or
'stty echo' after run to fix the terminal.

This script involves 3 levels of subshells.
[1] pipeline with a while loop.
 `--[2] command substitution in ask() key=$(...)
     `--[3] getch() body

Let's see what happens when the script is run (my comments inlined):

$ ./pickthing test
18406: pipe:[4898025] /dev/pts/1 /dev/pts/1 # toplevel shell
18412: pipe:[4898032] /dev/pts/1 /dev/pts/1 # [1]
Take item a? (y/n)
18417: pipe:[4898032] /dev/pts/1 /dev/pts/1 # [2] got stdin from [1]
18421: /dev/pts/1 /dev/pts/1 /dev/pts/1     # [3] got FD#4 as stdin
# I answered n here
18421: /dev/pts/1 /dev/pts/1 /dev/pts/1     # [3] last line
18421: pipe:[4898032] /dev/pts/1 /dev/pts/1 # [3] EXIT trap. WTF???
stty: standard input: Inappropriate ioctl for device

# Since I answered no, second iteration happened.

18412: pipe:[4898032] /dev/pts/1 /dev/pts/1 # [1]
Take item b? (y/n)
18439: pipe:[4898032] /dev/pts/1 /dev/pts/1 # [2]
18443: /dev/pts/1 /dev/pts/1 /dev/pts/1     # [3]
# I answered n here
18443: /dev/pts/1 /dev/pts/1 /dev/pts/1     # [3]
18443: pipe:[4898032] /dev/pts/1 /dev/pts/1 # [3] WTF???
stty: standard input: Inappropriate ioctl for device
Take test (initial literal value)? (y/n)

# It's the last line ask, so no pipeline here anymore.

18461: pipe:[4898099] /dev/pts/1 /dev/pts/1 # [2]
18465: /dev/pts/1 /dev/pts/1 /dev/pts/1     # [3]
18465: /dev/pts/1 /dev/pts/1 /dev/pts/1     # [3]
18465: pipe:[4898118] /dev/pts/1 /dev/pts/1 # [3] WTF???
stty: standard input: Bad file descriptor   # WTF?!?!?

The problem here is that although getch is run in a subshell and it
got FD#4 as stdin it somehow has parent's stdin in its EXIT trap.

POSIX says: "The environment in which the shell executes a trap on
EXIT shall be identical to the environment immediately after the
last command executed before the trap on EXIT was taken."

As you can see the last command of getch is dump_self_fds and stdin
is OK at that time.

In bash this script works as expected. For dash I had to invent
a workaround (commented out line key=$(...)).

-- 
Thanks,
Vitaly Sinilin
(I am not subscribed to the list, so please CC me.)

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [BUG] Redirection bug in subshell's EXIT trap
  2017-10-19 17:44 [BUG] Redirection bug in subshell's EXIT trap Vitaly Sinilin
@ 2017-10-19 21:57 ` Harald van Dijk
  0 siblings, 0 replies; 2+ messages in thread
From: Harald van Dijk @ 2017-10-19 21:57 UTC (permalink / raw)
  To: Vitaly Sinilin, dash

On 19/10/17 19:44, Vitaly Sinilin wrote:
> Hi,
> 
> I've recently written a quite unusual script and faced a strange dash
> behavior that seems not POSIX-ly correct to me.
> 
> Here is the script with a lot of debugging extra output.
> 
> (I am sorry about posting a pretty lengthy script, but I feel like a
> shorten version will look just insane without the context.)

A shortened version is easier to debug:

   dash -c 'f()(trap cat EXIT);f<&-'

This is supposed to print error messages. With dash, it doesn't.

> The problem here is that although getch is run in a subshell and it
> got FD#4 as stdin it somehow has parent's stdin in its EXIT trap.
> 
> POSIX says: "The environment in which the shell executes a trap on
> EXIT shall be identical to the environment immediately after the
> last command executed before the trap on EXIT was taken."

Yes, it is a bug. The problem is that dash, just before checking whether 
to execute the EXIT handler, has already reset everything to continue 
parsing and evaluating the next command. Which is not supposed to 
happen, never going to happen, and messes up the execution of the handler.

This resetting is done by the reset() function, and although the simple 
fix would be to move its call after exitshell(), the old ChangeLog shows 
that it was originally (about 15 years ago) the other way around and 
intentionally switched to how it is now, because some of what reset() 
does is necessary even here:

>  -- Herbert Xu <herbert@debian.org>  Sat, 26 Oct 2002 21:28:33 +1000
> 
> dash (0.4.2) unstable; urgency=low
> 
>   [...]
>   * Call reset() before exitshell() is called.  This fixes the bug where
>     returning an error from a function running under set -e caused the exit
>     trap to be taken with evalskip set.

Aside from the issue with redirections, the current order has other 
problems too:

   dash -c 'f()(local cmd="echo good";trap \$cmd EXIT);cmd="echo bad";f'

This is supposed to print "good". With dash, it prints "bad". With the 
call to reset() moved after exitshell(), it prints "good".

I suspect reset() needs to be split into two separate functions, but it 
may be a bit tricky to determine exactly what is supposed to go where.

Cheers,
Harald van Dijk

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2017-10-19 22:06 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-19 17:44 [BUG] Redirection bug in subshell's EXIT trap Vitaly Sinilin
2017-10-19 21:57 ` Harald van Dijk

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).