* dash 0.5.11.2, busybox sh 1.32.0, FreeBSD 12.2 sh: spring TTOU but should not i think
@ 2020-12-19 17:28 Steffen Nurpmeso
2020-12-19 22:21 ` Steffen Nurpmeso
0 siblings, 1 reply; 13+ messages in thread
From: Steffen Nurpmeso @ 2020-12-19 17:28 UTC (permalink / raw)
To: DASH shell mailing list, Denys Vlasenko, Jilles Tjoelker
[-- Attachment #1: Type: text/plain, Size: 1149 bytes --]
Hello.
Long story short, after falsely accusing BSD make of not working
when child shells use monitor mode someone pointed me into the
correct direction, and whereas the major problem was on the mksh
side (that sneaked in via $SHELL even though .. but regardless)
and was already reported, i can report a minor issue with the
mentioned shells nonetheless: with the reproducer (please do not
complain, it tooks hours to get it that short), when run under
supervision of a(ny tested) make(1), they spring a TTOU trap even
though they should not .. i think.
Please run via "make -f jobo.mk SHELL=YOUR-SHELL", and you should
see something like
./mx-test.sh
Starting job reaper (timeout of 2 seconds)
.. waiting for job reaper to come up
TTOU
....
where this TTOU should not happen i would say. It does not happen
with bash nor mksh nor OpenBSD sh and ksh, nor NetBSD sh and eh,
ksh, hmm.
I hope this helps, a nice weekend from Germany i wish,
--steffen
|
|Der Kragenbaer, The moon bear,
|der holt sich munter he cheerfully and one by one
|einen nach dem anderen runter wa.ks himself off
|(By Robert Gernhardt)
[-- Attachment #2: mx-test.sh --]
[-- Type: application/x-sh, Size: 10098 bytes --]
#!/bin/sh -
OBJDIR='/tmp/jobo';export OBJDIR # MANAGED!!
: ${SHELL:=/bin/sh};export SHELL
: ${JOBNO:=1}
: ${JOBWAIT:=2}
: ${JOBMON:=y}
export \
OBJDIR SHELL JOBNO JOBWAIT JOBMON
LC_ALL=C LANG=C TZ=UTC
export LC_ALL LANG TZ
if [ -z "${MAILX__CC_TEST_RUNNING}" ]; then
CHECK=1 RUN_TEST= MAILX=
export CHECK RUN_TEST MAILX
fi
awk='/usr/bin/awk';export awk
cat='/bin/cat';export cat
grep='/usr/bin/grep';export grep
mkdir='/bin/mkdir';export mkdir
rm='/bin/rm';export rm
if [ -z "${MAILX__CC_TEST_RUNNING}" ]; then
MAILX__CC_TEST_RUNNING=y
export MAILX__CC_TEST_RUNNING
rm -rf $OBJDIR
mkdir -p $OBJDIR
exec "${SHELL}" "${0}" "${@}"
fi
cd $OBJDIR || exit 100
DEVELDIFF= DUMPERR=
TESTS_PERFORMED=0 TESTS_OK=0 TESTS_FAILED=0 TESTS_SKIPPED=0
JOBS=0 JOBLIST= JOBDESC= JOBREAPER= JOBSYNC=
SUBSECOND_SLEEP=
( sleep .1 ) >/dev/null 2>&1 && SUBSECOND_SLEEP=y
TESTS_NET_TEST=
[ "${OPT_NET_TEST}" = 1 ] && [ -x ./net-test ] && TESTS_NET_TEST=1
export TESTS_NET_TEST
COLOR_ERR_ON= COLOR_ERR_OFF= COLOR_DBGERR_ON= COLOR_DBGERR_OFF=
COLOR_WARN_ON= COLOR_WARN_OFF=
COLOR_OK_ON= COLOR_OK_OFF=
ESTAT=0
TEST_NAME=
trap "
jobreaper_stop
" EXIT
trap "exit 1" HUP INT QUIT TERM
trap 'echo TTIN' TTIN
trap 'echo TTOU' TTOU
trap 'echo TSTP' TSTP
trap : CHLD
jobreaper_start() {
printf 'Starting job reaper (timeout of %s seconds)\n' ${JOBWAIT}
i=
trap 'i=1' USR1 # "reaper (actually a notify timer only) is up"
(
parent=${$}
sleeper= int=0 hot=0
trap '' EXIT HUP QUIT CHLD
trap 'exit 0' INT
trap '
[ -n "${sleeper}" ] && kill -TERM ${sleeper} >/dev/null 2>&1
int=1 hot=1
' USR1
trap '
[ -n "${sleeper}" ] && kill -TERM ${sleeper} >/dev/null 2>&1
int=1 hot=0
' USR2
trap '
[ -n "${sleeper}" ] && kill -TERM ${sleeper} >/dev/null 2>&1
echo "Stopping job reaper"
exit 0
' TERM
# traps are setup, notify parent that we are up and running
kill -USR1 ${parent} >/dev/null 2>&1
while :; do
int=0
sleep ${JOBWAIT} &
sleeper=${!}
wait ${!}
sleeper=
[ "${int}${hot}" = 01 ] && kill -USR1 ${parent} >/dev/null 2>&1
done
) </dev/null & #>/dev/null 2>&1 &
JOBREAPER=${!}
j=
if [ ${?} -eq 0 ]; then
while :; do
if [ -n "${i}" ]; then
trap '' USR1
return
fi
printf '..%s waiting for job reaper to come up\n' "${j}"
j=' still'
sleep 1 &
wait ${!}
done
fi
JOBREAPER=
printf '%s!! Cannot start the wild job reaper!%s\n' \
"${COLOR_ERR_ON}" "${COLOR_ERR_OFF}"
}
jobreaper_stop() {
[ -n "${JOBREAPER}" ] && kill -TERM ${JOBREAPER} >/dev/null 2>&1
JOBREAPER=
if [ ${JOBS} -gt 0 ]; then
echo 'Cleaning up running jobs'
jtimeout
wait ${JOBLIST}
JOBLIST=
fi
}
jspawn() {
if [ ${JOBNO} -gt 1 ]; then
# We are spawning multiple jobs..
[ ${JOBS} -eq 0 ] && printf '...'
JOBS=`add ${JOBS} 1`
printf ' [%s=%s]' ${JOBS} "${1}"
else
JOBS=1
# Assume problems exist, do not let user keep hanging on terminal
if [ -n "${RUN_TEST}" ]; then
printf '... [%s]\n' "${1}"
fi
fi
[ -n "${JOBMON}" ] && set -m >/dev/null 2>&1
( # Place the job in its own directory to ease file management
trap '' EXIT HUP INT QUIT TERM USR1 USR2
${mkdir} t.${JOBS}.d && cd t.${JOBS}.d &&
eval t_${1} ${JOBS} ${1} &&
${rm} -f ../t.${JOBS}.id
) > t.${JOBS}.io </dev/null & # 2>&1 </dev/null &
i=${!}
[ -n "${JOBMON}" ] && set +m >/dev/null 2>&1
JOBLIST="${JOBLIST} ${i}"
printf '%s\n%s\n' ${i} ${1} > t.${JOBS}.id
# ..until we should sync or reach the maximum concurrent number
[ ${JOBS} -lt ${JOBNO} ] && return
jsync 1
}
jsync() {
if [ ${JOBS} -eq 0 ]; then
[ -n "${TEST_ANY}" ] && printf '\n'
TEST_ANY=
return
fi
[ -z "${JOBSYNC}" ] && [ ${#} -eq 0 ] && return
[ ${JOBNO} -ne 1 ] && printf ' .. waiting\n'
if [ -n "${JOBREAPER}" ]; then
timeout= alldone=
trap 'echo TIMEOUT IN PARENT;timeout=1' USR1
kill -USR1 ${JOBREAPER} >/dev/null 2>&1
loops=0
while [ -z "${timeout}" ]; do
alldone=1
i=0
while [ ${i} -lt ${JOBS} ]; do
i=`add ${i} 1`
[ -f t.${i}.id ] || continue
alldone=
break
done
[ -n "${alldone}" ] && break
if [ -z "${SUBSECOND_SLEEP}" ]; then
loops=`add ${loops} 1`
[ ${loops} -lt 111 ] && continue
sleep 1 &
else
sleep .5 &
fi
echo >&2 PREWAIT
wait ${!}
done
kill -USR2 ${JOBREAPER} >/dev/null 2>&1
trap '' USR1
[ -n "${timeout}" ] && jtimeout
fi
# Now collect the zombies
wait ${JOBLIST}
JOBLIST=
# Update global counters
i=0
while [ ${i} -lt ${JOBS} ]; do
i=`add ${i} 1`
if [ -f t.${i}.id ]; then
{ read pid; read desc; } < t.${i}.id
desc=${desc#${desc%%[! ]*}}
desc=${desc%${desc##*[! ]}}
[ -s t.${i}.io ] && printf >&2 '\n'
printf >&2 '%s!! Timeout: reaped job %s [%s]%s\n' \
"${COLOR_ERR_ON}" ${i} "${desc}" "${COLOR_ERR_OFF}"
TESTS_FAILED=`add ${TESTS_FAILED} 1`
elif [ -s t.${i}.result ]; then
read es tp to tf ts < t.${i}.result
TESTS_PERFORMED=`add ${TESTS_PERFORMED} ${tp}`
TESTS_OK=`add ${TESTS_OK} ${to}`
TESTS_FAILED=`add ${TESTS_FAILED} ${tf}`
TESTS_SKIPPED=`add ${TESTS_SKIPPED} ${ts}`
[ "${es}" != 0 ] && ESTAT=${es}
else
TESTS_FAILED=`add ${TESTS_FAILED} 1`
ESTAT=1
fi
done
JOBS=0
}
jtimeout() {
i=0
while [ ${i} -lt ${JOBS} ]; do
i=`add ${i} 1`
if [ -f t.${i}.id ] &&
read pid < t.${i}.id >/dev/null 2>&1 &&
kill -0 ${pid} >/dev/null 2>&1; then
j=${pid}
[ -n "${JOBMON}" ] && j=-${j}
kill -KILL ${j} >/dev/null 2>&1
else
${rm} -f t.${i}.id
fi
done
}
t_prolog() {
shift
ESTAT=0 TESTS_PERFORMED=0 TESTS_OK=0 TESTS_FAILED=0 TESTS_SKIPPED=0 \
TEST_NAME=${1} TEST_ANY=
printf '%s[%s]%s\n' "" "${TEST_NAME}" ""
}
t_epilog() {
[ -n "${TEST_ANY}" ] && printf '\n'
printf '%s %s %s %s %s\n' \
${ESTAT} \
${TESTS_PERFORMED} ${TESTS_OK} ${TESTS_FAILED} ${TESTS_SKIPPED} \
> ../t.${1}.result
}
t_echo() {
[ -n "${TEST_ANY}" ] && __i__=' ' || __i__=
printf "${__i__}"'%s' "${*}"
TEST_ANY=1
}
t_echook() {
[ -n "${TEST_ANY}" ] && __i__=' ' || __i__=
printf "${__i__}"'%s%s:ok%s' "${COLOR_OK_ON}" "${*}" "${COLOR_OK_OFF}"
TEST_ANY=1
}
t_echoerr() {
ESTAT=1
t_echo0err "${@}"
}
t_echo0err() {
[ -n "${TEST_ANY}" ] && __i__="\n" || __i__=
printf "${__i__}"'%sERROR: %s%s\n' \
"${COLOR_ERR_ON}" "${*}" "${COLOR_ERR_OFF}"
TEST_ANY=
}
t_echowarn() {
[ -n "${TEST_ANY}" ] && __i__=' ' || __i__=
printf "${__i__}"'%s%s%s' "${COLOR_WARN_ON}" "${*}" "${COLOR_WARN_OFF}"
TEST_ANY=1
}
t_echoskip() {
[ -n "${TEST_ANY}" ] && __i__=' ' || __i__=
printf "${__i__}"'%s%s[skip]%s' \
"${COLOR_WARN_ON}" "${*}" "${COLOR_WARN_OFF}"
TEST_ANY=1
TESTS_SKIPPED=`add ${TESTS_SKIPPED} 1`
}
check() {
restat=${?} tid=${1} eestat=${2} f=${3} s=${4} optmode=${5}
echo >&2 CHECK $TID 0
TESTS_PERFORMED=`add ${TESTS_PERFORMED} 1`
echo >&2 CHECK $TID 10
if [ -n "${CHECK}${RUN_TEST}" ]; then
x="t.${TEST_NAME}-${tid}"
echo >&2 CHECK $TID 11
if :; then
y=test-out
echo >&2 CHECK $TID 20
echo >&2 CHECK $TID 21
if [ -n "${y}" ]; then
echo >&2 CHECK $TID 22
if : ; then
echo >&2 CHECK $TID 22.5
# NOTE: REMOVE THIS (or use true(1)) AND WORKS directly and via bmake (mksh)
# (env -i regardless, "false" also is not it, whatever)
env -i ls -latr /NONEXISTENT
echo >&2 CHECK $SHELL $TID 22.8
# NOTE: monitor should be "on" here
set -o >&2
echo >&2 CHECK $TID 23
fi
echo >&2 CHECK $TID 30
fi
echo >&2 CHECK $TID 35
fi
fi
echo >&2 CHECK $TID 100
}
check_ex0() {
# $1=test name [$2=status]
__qm__=${?}
[ ${#} -gt 1 ] && __qm__=${2}
TESTS_PERFORMED=`add ${TESTS_PERFORMED} 1`
if [ ${__qm__} -ne 0 ]; then
ESTAT=1
t_echoerr "${1}: unexpected non-0 exit status: ${__qm__}"
TESTS_FAILED=`add ${TESTS_FAILED} 1`
else
t_echook "${1}"
TESTS_OK=`add ${TESTS_OK} 1`
fi
}
check_exn0() {
# $1=test name [$2=status]
__qm__=${?}
[ ${#} -gt 1 ] && __qm__=${2}
[ ${#} -gt 2 ] && __expect__=${3} || __expect__=
TESTS_PERFORMED=`add ${TESTS_PERFORMED} 1`
if [ ${__qm__} -eq 0 ]; then
ESTAT=1
t_echoerr "${1}: unexpected 0 exit status: ${__qm__}"
TESTS_FAILED=`add ${TESTS_FAILED} 1`
elif [ -n "${__expect__}" ] && [ ${__expect__} -ne ${__qm__} ]; then
ESTAT=1
t_echoerr "${1}: unexpected exit status: ${__qm__} != ${__expected__}"
TESTS_FAILED=`add ${TESTS_FAILED} 1`
else
t_echook "${1}"
TESTS_OK=`add ${TESTS_OK} 1`
fi
}
color_init() {
:
}
if ( [ "$((1 + 1))" = 2 ] ) >/dev/null 2>&1; then
add() {
echo "$((${1} + ${2}))"
}
else
add() {
${awk} 'BEGIN{print '${1}' + '${2}'}'
}
fi
if ( [ "$((2 % 3))" = 2 ] ) >/dev/null 2>&1; then
modulo() {
echo "$((${1} % ${2}))"
}
else
modulo() {
${awk} 'BEGIN{print '${1}' % '${2}'}'
}
fi
have_feat() {
( "${RAWMAILX}" ${ARGS} -X'echo $features' -Xx |
${grep} +${1}, ) >/dev/null 2>&1
}
t_X_errexit() {
t_prolog "${@}"
</dev/null ${MAILX} ${ARGS} -Snomemdebug \
-X'echo one' -X' echos nono ' -X'echo two' \
> ./t1 2>&1
echo >&2 PRE T1 CHEK
check 1 0 ./t1 '2700500141 51'
echo >&2 AFTER T1 CHEK
t_epilog "$@"
return
}
ssec=$SECONDS
jobreaper_start
jspawn X_errexit
jsync 1
jobreaper_stop
esec=$SECONDS
exit ${ESTAT}
# s-sh-mode
[-- Attachment #3: jobo.mk --]
[-- Type: text/plain, Size: 20 bytes --]
test:
./mx-test.sh
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: dash 0.5.11.2, busybox sh 1.32.0, FreeBSD 12.2 sh: spring TTOU but should not i think
2020-12-19 17:28 dash 0.5.11.2, busybox sh 1.32.0, FreeBSD 12.2 sh: spring TTOU but should not i think Steffen Nurpmeso
@ 2020-12-19 22:21 ` Steffen Nurpmeso
2020-12-19 23:52 ` Harald van Dijk
0 siblings, 1 reply; 13+ messages in thread
From: Steffen Nurpmeso @ 2020-12-19 22:21 UTC (permalink / raw)
To: DASH shell mailing list, Denys Vlasenko, Jilles Tjoelker
[-- Attachment #1: Type: text/plain, Size: 2528 bytes --]
Steffen Nurpmeso wrote in
<20201219172838.1B-WB%steffen@sdaoden.eu>:
|Long story short, after falsely accusing BSD make of not working
After dinner i shortened it a bit more, and attach it again, ok?
It is terrible, but now less redundant than before.
Sorry for being so terse, that problem crosses my head for about
a week, and i was totally mislead and if you bang your head
against the wall so many hours bugs or misbehaviours in a handful
of other programs is not the expected outcome.
#?0|kent:tmp$ SHELL=/bin/bash make -f jobo.mk
./mx-test.sh
Starting job reaper (timeout of 2 seconds)
.. waiting for job reaper to come up
PRE T1 CHEK
PREWAIT
CHECK 10
ls: cannot access '/NONEXISTENT': No such file or directory
CHECK /bin/bash 50
allexport off
braceexpand on
emacs off
errexit off
errtrace off
functrace off
hashall on
histexpand off
history off
ignoreeof off
interactive-comments on
keyword off
monitor on
noclobber off
noexec off
noglob off
nolog off
notify off
nounset off
onecmd off
physical off
pipefail off
posix off
privileged off
verbose off
vi off
xtrace off
CHECK 23
CHECK 100
AFTER T1 CHEK
Stopping job reaper
make: *** [jobo.mk:2: test] Error 1
That is how it "should look" ;), monitor is on in there.
#?2|kent:tmp$ SHELL=/bin/dash make -f jobo.mk
./mx-test.sh
Starting job reaper (timeout of 2 seconds)
.. waiting for job reaper to come up
TTOU
Cleaning up running jobs
Stopping job reaper
make: *** [jobo.mk:2: test] Error 2
Somehow refuses to work when supervised by (any) make (i tried).
Works as desired when ridden directly.
#?2|kent:tmp$ SHELL=/bin/mksh make -f jobo.mk
./mx-test.sh
Starting job reaper (timeout of 2 seconds)
.. waiting for job reaper to come up
PRE T1 CHEK
CHECK 10
PREWAIT
ls: cannot access '/NONEXISTENT': No such file or directory
PREWAIT
PREWAIT
PREWAIT
PREWAIT
PREWAIT
PREWAIT
PREWAIT
TIMEOUT IN PARENT
!! Timeout: reaped job 1 [X_errexit]
Stopping job reaper
And that condition is very strange, with or without supervision.
Ciao!
--steffen
|
|Der Kragenbaer, The moon bear,
|der holt sich munter he cheerfully and one by one
|einen nach dem anderen runter wa.ks himself off
|(By Robert Gernhardt)
[-- Attachment #2: mx-test.sh --]
[-- Type: application/x-sh, Size: 7347 bytes --]
#!/bin/sh -
OBJDIR='/tmp/jobo';export OBJDIR # MANAGED!!
: ${SHELL:=/bin/sh};export SHELL
: ${JOBNO:=1}
: ${JOBWAIT:=2}
: ${JOBMON:=y}
export \
OBJDIR SHELL JOBNO JOBWAIT JOBMON
LC_ALL=C LANG=C TZ=UTC
export LC_ALL LANG TZ
if [ -z "${MAILX__CC_TEST_RUNNING}" ]; then
CHECK=1 RUN_TEST= MAILX=
export CHECK RUN_TEST MAILX
fi
mkdir='/bin/mkdir';export mkdir
rm='/bin/rm';export rm
if [ -z "${MAILX__CC_TEST_RUNNING}" ]; then
MAILX__CC_TEST_RUNNING=y
export MAILX__CC_TEST_RUNNING
rm -rf $OBJDIR
mkdir -p $OBJDIR
exec "${SHELL}" "${0}" "${@}"
fi
cd $OBJDIR || exit 100
DEVELDIFF= DUMPERR=
TESTS_PERFORMED=0 TESTS_OK=0 TESTS_FAILED=0 TESTS_SKIPPED=0
JOBS=0 JOBLIST= JOBDESC= JOBREAPER= JOBSYNC=
SUBSECOND_SLEEP=
( sleep .1 ) >/dev/null 2>&1 && SUBSECOND_SLEEP=y
TESTS_NET_TEST=
[ "${OPT_NET_TEST}" = 1 ] && [ -x ./net-test ] && TESTS_NET_TEST=1
export TESTS_NET_TEST
COLOR_ERR_ON= COLOR_ERR_OFF= COLOR_DBGERR_ON= COLOR_DBGERR_OFF=
COLOR_WARN_ON= COLOR_WARN_OFF=
COLOR_OK_ON= COLOR_OK_OFF=
ESTAT=0
TEST_NAME=
trap "
jobreaper_stop
" EXIT
trap "exit 1" HUP INT QUIT TERM
trap 'echo TTIN' TTIN
trap 'echo TTOU' TTOU
trap 'echo TSTP' TSTP
trap : CHLD
jobreaper_start() {
printf 'Starting job reaper (timeout of %s seconds)\n' ${JOBWAIT}
i=
trap 'i=1' USR1 # "reaper (actually a notify timer only) is up"
(
parent=${$}
sleeper= int=0 hot=0
trap '' EXIT HUP QUIT CHLD
trap 'exit 0' INT
trap '
[ -n "${sleeper}" ] && kill -TERM ${sleeper} >/dev/null 2>&1
int=1 hot=1
' USR1
trap '
[ -n "${sleeper}" ] && kill -TERM ${sleeper} >/dev/null 2>&1
int=1 hot=0
' USR2
trap '
[ -n "${sleeper}" ] && kill -TERM ${sleeper} >/dev/null 2>&1
echo "Stopping job reaper"
exit 0
' TERM
# traps are setup, notify parent that we are up and running
kill -USR1 ${parent} >/dev/null 2>&1
while :; do
int=0
sleep ${JOBWAIT} &
sleeper=${!}
wait ${!}
sleeper=
[ "${int}${hot}" = 01 ] && kill -USR1 ${parent} >/dev/null 2>&1
done
) </dev/null & #>/dev/null 2>&1 &
JOBREAPER=${!}
j=
if [ ${?} -eq 0 ]; then
while :; do
if [ -n "${i}" ]; then
trap '' USR1
return
fi
printf '..%s waiting for job reaper to come up\n' "${j}"
j=' still'
sleep 1 &
wait ${!}
done
fi
JOBREAPER=
printf '%s!! Cannot start the wild job reaper!%s\n' \
"${COLOR_ERR_ON}" "${COLOR_ERR_OFF}"
}
jobreaper_stop() {
[ -n "${JOBREAPER}" ] && kill -TERM ${JOBREAPER} >/dev/null 2>&1
JOBREAPER=
if [ ${JOBS} -gt 0 ]; then
echo 'Cleaning up running jobs'
jtimeout
wait ${JOBLIST}
JOBLIST=
fi
}
jspawn() {
if [ ${JOBNO} -gt 1 ]; then
# We are spawning multiple jobs..
[ ${JOBS} -eq 0 ] && printf '...'
JOBS=`add ${JOBS} 1`
printf ' [%s=%s]' ${JOBS} "${1}"
else
JOBS=1
# Assume problems exist, do not let user keep hanging on terminal
if [ -n "${RUN_TEST}" ]; then
printf '... [%s]\n' "${1}"
fi
fi
[ -n "${JOBMON}" ] && set -m >/dev/null 2>&1
( # Place the job in its own directory to ease file management
trap '' EXIT HUP INT QUIT TERM USR1 USR2
#${mkdir} t.${JOBS}.d && cd t.${JOBS}.d &&
eval t_${1} ${JOBS} ${1} &&
${rm} -f t.${JOBS}.id
) > t.${JOBS}.io </dev/null & # 2>&1 </dev/null &
i=${!}
[ -n "${JOBMON}" ] && set +m >/dev/null 2>&1
JOBLIST="${JOBLIST} ${i}"
printf '%s\n%s\n' ${i} ${1} > t.${JOBS}.id
# ..until we should sync or reach the maximum concurrent number
[ ${JOBS} -lt ${JOBNO} ] && return
jsync 1
}
jsync() {
if [ ${JOBS} -eq 0 ]; then
[ -n "${TEST_ANY}" ] && printf '\n'
TEST_ANY=
return
fi
[ -z "${JOBSYNC}" ] && [ ${#} -eq 0 ] && return
[ ${JOBNO} -ne 1 ] && printf ' .. waiting\n'
if [ -n "${JOBREAPER}" ]; then
timeout= alldone=
trap 'echo TIMEOUT IN PARENT;timeout=1' USR1
kill -USR1 ${JOBREAPER} >/dev/null 2>&1
loops=0
while [ -z "${timeout}" ]; do
alldone=1
i=0
while [ ${i} -lt ${JOBS} ]; do
i=`add ${i} 1`
[ -f t.${i}.id ] || continue
alldone=
break
done
[ -n "${alldone}" ] && break
if [ -z "${SUBSECOND_SLEEP}" ]; then
loops=`add ${loops} 1`
[ ${loops} -lt 111 ] && continue
sleep 1 &
else
sleep .5 &
fi
echo >&2 PREWAIT
wait ${!}
done
kill -USR2 ${JOBREAPER} >/dev/null 2>&1
trap '' USR1
[ -n "${timeout}" ] && jtimeout
fi
# Now collect the zombies
wait ${JOBLIST}
JOBLIST=
# Update global counters
i=0
while [ ${i} -lt ${JOBS} ]; do
i=`add ${i} 1`
if [ -f t.${i}.id ]; then
{ read pid; read desc; } < t.${i}.id
desc=${desc#${desc%%[! ]*}}
desc=${desc%${desc##*[! ]}}
[ -s t.${i}.io ] && printf >&2 '\n'
printf >&2 '%s!! Timeout: reaped job %s [%s]%s\n' \
"${COLOR_ERR_ON}" ${i} "${desc}" "${COLOR_ERR_OFF}"
TESTS_FAILED=`add ${TESTS_FAILED} 1`
else
TESTS_FAILED=`add ${TESTS_FAILED} 1`
ESTAT=1
fi
done
JOBS=0
}
jtimeout() {
i=0
while [ ${i} -lt ${JOBS} ]; do
i=`add ${i} 1`
if [ -f t.${i}.id ] &&
read pid < t.${i}.id >/dev/null 2>&1 &&
kill -0 ${pid} >/dev/null 2>&1; then
j=${pid}
[ -n "${JOBMON}" ] && j=-${j}
kill -KILL ${j} >/dev/null 2>&1
else
${rm} -f t.${i}.id
fi
done
}
t_prolog() {
shift
ESTAT=0 TESTS_PERFORMED=0 TESTS_OK=0 TESTS_FAILED=0 TESTS_SKIPPED=0 \
TEST_NAME=${1} TEST_ANY=
printf '%s[%s]%s\n' "" "${TEST_NAME}" ""
}
t_epilog() {
[ -n "${TEST_ANY}" ] && printf '\n'
printf '%s %s %s %s %s\n' \
${ESTAT} \
${TESTS_PERFORMED} ${TESTS_OK} ${TESTS_FAILED} ${TESTS_SKIPPED} \
> ../t.${1}.result
}
t_echoerr() {
ESTAT=1
t_echo0err "${@}"
}
t_echo0err() {
[ -n "${TEST_ANY}" ] && __i__="\n" || __i__=
printf "${__i__}"'%sERROR: %s%s\n' \
"${COLOR_ERR_ON}" "${*}" "${COLOR_ERR_OFF}"
TEST_ANY=
}
check() {
restat=${?} tid=${1} eestat=${2} f=${3} s=${4} optmode=${5}
TESTS_PERFORMED=`add ${TESTS_PERFORMED} 1`
if [ -n "${CHECK}${RUN_TEST}" ]; then
x="t.${TEST_NAME}-${tid}"
if :; then
y=test-out
if [ -n "${y}" ]; then
if : ; then
echo >&2 CHECK $TID 10
# NOTE: REMOVE THIS (or use true(1)) AND WORKS directly and via bmake (mksh)
# (env -i regardless, "false" also is not it, whatever)
env -i ls -latr /NONEXISTENT
echo >&2 CHECK $SHELL $TID 50
# NOTE: monitor should be "on" here
set -o >&2
echo >&2 CHECK $TID 23
fi
fi
fi
fi
echo >&2 CHECK $TID 100
}
add() {
echo "$((${1} + ${2}))"
}
t_X_errexit() {
t_prolog "${@}"
echo >&2 PRE T1 CHEK
check 1 0 ./t1 '2700500141 51'
echo >&2 AFTER T1 CHEK
t_epilog "$@"
return
}
ssec=$SECONDS
jobreaper_start
jspawn X_errexit
jsync 1
jobreaper_stop
esec=$SECONDS
exit ${ESTAT}
# s-sh-mode
[-- Attachment #3: jobo.mk --]
[-- Type: text/plain, Size: 20 bytes --]
test:
./mx-test.sh
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: dash 0.5.11.2, busybox sh 1.32.0, FreeBSD 12.2 sh: spring TTOU but should not i think
2020-12-19 22:21 ` Steffen Nurpmeso
@ 2020-12-19 23:52 ` Harald van Dijk
2020-12-21 16:24 ` Jilles Tjoelker
2021-01-06 4:45 ` [PATCH] jobs: Block signals during tcsetpgrp Herbert Xu
0 siblings, 2 replies; 13+ messages in thread
From: Harald van Dijk @ 2020-12-19 23:52 UTC (permalink / raw)
To: Steffen Nurpmeso, DASH shell mailing list, Denys Vlasenko,
Jilles Tjoelker
On 19/12/2020 22:21, Steffen Nurpmeso wrote:
> Steffen Nurpmeso wrote in
> <20201219172838.1B-WB%steffen@sdaoden.eu>:
> |Long story short, after falsely accusing BSD make of not working
>
> After dinner i shortened it a bit more, and attach it again, ok?
> It is terrible, but now less redundant than before.
> Sorry for being so terse, that problem crosses my head for about
> a week, and i was totally mislead and if you bang your head
> against the wall so many hours bugs or misbehaviours in a handful
> of other programs is not the expected outcome.
I think a minimal test case is simply
all:
$(SHELL) -c 'trap "echo TTOU" TTOU; set -m; echo all good'
unless I accidentally oversimplified.
The SIGTTOU is caused by setjobctl's xtcsetpgrp(fd, pgrp) call to make
its newly started process group the foreground process group when job
control is enabled, where xtcsetpgrp is a wrapper for tcsetpgrp. (That's
in dash, the other variants may have some small differences.) tcsetpgrp
has this little bit in its specification:
Attempts to use tcsetpgrp() from a process which is a member of
a background process group on a fildes associated with its con‐
trolling terminal shall cause the process group to be sent a
SIGTTOU signal. If the calling thread is blocking SIGTTOU sig‐
nals or the process is ignoring SIGTTOU signals, the process
shall be allowed to perform the operation, and no signal is
sent.
Ordinarily, when job control is enabled, SIGTTOU is ignored. However,
when a trap action is specified for SIGTTOU, the signal is not ignored,
and there is no blocking in place either, so the tcsetpgrp() call is not
allowed.
The lowest impact change to make here, the one that otherwise preserves
the existing shell behaviour, is to block signals before calling
tcsetpgrp and unblocking them afterwards. This ensures SIGTTOU does not
get raised here, but also ensures that if SIGTTOU is sent to the shell
for another reason, there is no window where it gets silently ignored.
Another way to fix this is by not trying to make the shell start a new
process group, or at least not make it the foreground process group.
Most other shells appear to not try to do this.
Cheers,
Harald van Dijk
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: dash 0.5.11.2, busybox sh 1.32.0, FreeBSD 12.2 sh: spring TTOU but should not i think
2020-12-19 23:52 ` Harald van Dijk
@ 2020-12-21 16:24 ` Jilles Tjoelker
2020-12-21 19:43 ` Steffen Nurpmeso
` (2 more replies)
2021-01-06 4:45 ` [PATCH] jobs: Block signals during tcsetpgrp Herbert Xu
1 sibling, 3 replies; 13+ messages in thread
From: Jilles Tjoelker @ 2020-12-21 16:24 UTC (permalink / raw)
To: Harald van Dijk; +Cc: Steffen Nurpmeso, DASH shell mailing list, Denys Vlasenko
On Sat, Dec 19, 2020 at 11:52:31PM +0000, Harald van Dijk wrote:
> On 19/12/2020 22:21, Steffen Nurpmeso wrote:
> > Steffen Nurpmeso wrote in
> > <20201219172838.1B-WB%steffen@sdaoden.eu>:
> > |Long story short, after falsely accusing BSD make of not working
> > After dinner i shortened it a bit more, and attach it again, ok?
> > It is terrible, but now less redundant than before.
> > Sorry for being so terse, that problem crosses my head for about
> > a week, and i was totally mislead and if you bang your head
> > against the wall so many hours bugs or misbehaviours in a handful
> > of other programs is not the expected outcome.
> I think a minimal test case is simply
> all:
> $(SHELL) -c 'trap "echo TTOU" TTOU; set -m; echo all good'
> unless I accidentally oversimplified.
Yes, and it can be simplified a bit more to remove make from the
picture:
(SHELL -c 'trap "echo TTOU" TTOU; set -m; echo all good'; :)
The idea is to start the shell without being a process group leader, set
a trap for SIGTTOU and enable job control.
Also, simply entering the command
trap "echo TTOU" TTOU
in an interactive shell makes it behave strangely. Created jobs
immediately stop, "fg" works once but after that the shell tends to get
stuck quickly.
> The SIGTTOU is caused by setjobctl's xtcsetpgrp(fd, pgrp) call to make its
> newly started process group the foreground process group when job control is
> enabled, where xtcsetpgrp is a wrapper for tcsetpgrp. (That's in dash, the
> other variants may have some small differences.) tcsetpgrp has this little
> bit in its specification:
> Attempts to use tcsetpgrp() from a process which is a member of
> a background process group on a fildes associated with its con‐
> trolling terminal shall cause the process group to be sent a
> SIGTTOU signal. If the calling thread is blocking SIGTTOU sig‐
> nals or the process is ignoring SIGTTOU signals, the process
> shall be allowed to perform the operation, and no signal is
> sent.
> Ordinarily, when job control is enabled, SIGTTOU is ignored. However, when a
> trap action is specified for SIGTTOU, the signal is not ignored, and there
> is no blocking in place either, so the tcsetpgrp() call is not allowed.
Right.
> The lowest impact change to make here, the one that otherwise preserves the
> existing shell behaviour, is to block signals before calling tcsetpgrp and
> unblocking them afterwards. This ensures SIGTTOU does not get raised here,
> but also ensures that if SIGTTOU is sent to the shell for another reason,
> there is no window where it gets silently ignored.
This seems a good approach, but certain writes to the terminal may need
the same treatment, for example the error message when fork fails for
the second and further elements of a pipeline. This also makes me wonder
why SIGTTOU is ignored at all by default.
In mksh, the issue is resolved slightly differently: setting a trap on
TTOU pretends to work but the signal disposition remains set to ignored.
> Another way to fix this is by not trying to make the shell start a new
> process group, or at least not make it the foreground process group. Most
> other shells appear to not try to do this.
Tradition is for job control shells to be a process group leader, but I
don't really know why. Changing this will not fix the issue entirely
anyway since the shell must perform tcsetpgrp() from the background when
a foreground job has terminated.
What is definitely required, though, is that the shell not read input or
alter terminal settings if it is started in the background (possibly
unless the script explicitly ignored SIGTTOU). This is what the loop
with tcgetpgrp() does.
--
Jilles Tjoelker
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: dash 0.5.11.2, busybox sh 1.32.0, FreeBSD 12.2 sh: spring TTOU but should not i think
2020-12-21 16:24 ` Jilles Tjoelker
@ 2020-12-21 19:43 ` Steffen Nurpmeso
2020-12-23 20:18 ` Harald van Dijk
2021-01-06 4:46 ` Herbert Xu
2 siblings, 0 replies; 13+ messages in thread
From: Steffen Nurpmeso @ 2020-12-21 19:43 UTC (permalink / raw)
To: Jilles Tjoelker; +Cc: Harald van Dijk, DASH shell mailing list, Denys Vlasenko
Jilles Tjoelker wrote in
<20201221162442.GA26001@stack.nl>:
|On Sat, Dec 19, 2020 at 11:52:31PM +0000, Harald van Dijk wrote:
|> On 19/12/2020 22:21, Steffen Nurpmeso wrote:
|>> Steffen Nurpmeso wrote in
|>> <20201219172838.1B-WB%steffen@sdaoden.eu>:
|>>|Long story short, after falsely accusing BSD make of not working
|
|>> After dinner i shortened it a bit more, and attach it again, ok?
|>> It is terrible, but now less redundant than before.
|>> Sorry for being so terse, that problem crosses my head for about
|>> a week, and i was totally mislead and if you bang your head
|>> against the wall so many hours bugs or misbehaviours in a handful
|>> of other programs is not the expected outcome.
|
|> I think a minimal test case is simply
|
|> all:
|> $(SHELL) -c 'trap "echo TTOU" TTOU; set -m; echo all good'
|
|> unless I accidentally oversimplified.
|
|Yes, and it can be simplified a bit more to remove make from the
|picture:
| (SHELL -c 'trap "echo TTOU" TTOU; set -m; echo all good'; :)
Cool, .. but it all seems to depend on the outer shell
#$ bash -c "( /bin/dash -c 'trap echo\ TTOU TTOU; set -m; echo all good' )"
/bin/dash: 1: set: Cannot set tty process group (Interrupted system call)
#$ mksh -c "( /bin/dash -c 'trap echo\ TTOU TTOU; set -m; echo all good' )"
/bin/dash: 1: set: Cannot set tty process group (Interrupted system call)
#$ dash -c "( /bin/dash -c 'trap echo\ TTOU TTOU; set -m; echo all good' )"
all good
which results in the funny experience
#?0|kent:tmp$ smake -f t.mk SHELL=/bin/bash
.../bin/bash -c 'trap "echo TTOU" TTOU; set -m; echo all good'
all good
#?0|kent:tmp$ smake -f t.mk SHELL=/bin/mksh
.../bin/mksh -c 'trap "echo TTOU" TTOU; set -m; echo all good'
all good
#?0|kent:tmp$ smake -f t.mk SHELL=/bin/dash
.../bin/dash -c 'trap "echo TTOU" TTOU; set -m; echo all good'
/bin/dash: 1: set: Cannot set tty process group (Interrupted system call)
smake: *** Code 2 (No such file or directory) from command line for target 'all'.
smake: Couldn't make 'all'.
Ah, it is a Monday ..
--steffen
|
|Der Kragenbaer, The moon bear,
|der holt sich munter he cheerfully and one by one
|einen nach dem anderen runter wa.ks himself off
|(By Robert Gernhardt)
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: dash 0.5.11.2, busybox sh 1.32.0, FreeBSD 12.2 sh: spring TTOU but should not i think
2020-12-21 16:24 ` Jilles Tjoelker
2020-12-21 19:43 ` Steffen Nurpmeso
@ 2020-12-23 20:18 ` Harald van Dijk
2020-12-24 15:29 ` Jilles Tjoelker
2021-01-10 23:56 ` Harald van Dijk
2021-01-06 4:46 ` Herbert Xu
2 siblings, 2 replies; 13+ messages in thread
From: Harald van Dijk @ 2020-12-23 20:18 UTC (permalink / raw)
To: Jilles Tjoelker; +Cc: Steffen Nurpmeso, DASH shell mailing list, Denys Vlasenko
On 21/12/2020 16:24, Jilles Tjoelker wrote:
> Also, simply entering the command
> trap "echo TTOU" TTOU
> in an interactive shell makes it behave strangely. Created jobs
> immediately stop, "fg" works once but after that the shell tends to get
> stuck quickly.
Good catch, there is some work to be done there too.
> This seems a good approach, but certain writes to the terminal may need
> the same treatment, for example the error message when fork fails for
> the second and further elements of a pipeline. This also makes me wonder
> why SIGTTOU is ignored at all by default.
True. This is hard to create a reliable test case for, but there is only
limited code that can get executed while a job-control-enabled shell may
not be in the foreground process group.
If fork fails halfway through a pipeline, it may also be a problem that
some of the commands in the pipeline may still run.
> In mksh, the issue is resolved slightly differently: setting a trap on
> TTOU pretends to work but the signal disposition remains set to ignored.
zsh also rejects traps on TTOU, but does so explicitly:
zsh: can't trap SIGTTOU in interactive shells
Amusingly, it prints this in any shell where job control is enabled,
regardless of whether the shell is interactive.
> Tradition is for job control shells to be a process group leader, but I
> don't really know why. Changing this will not fix the issue entirely
> anyway since the shell must perform tcsetpgrp() from the background when
> a foreground job has terminated.
I've been thinking more about this, and I suspect it's a another
conflation between interactive mode and job control. In an interactive
shell that's not executing any external program, you want any Ctrl-C to
only send SIGINT to that shell, not to any other process. In order for
that to work, it needs to be its own process group.
This should, in my opinion, *only* happen for interactive shells, not
for job-control-enabled non-interactive shells. Consider
sh -c 'sh -mc "while :; do :; done"; echo bye'
The behaviour I would want is that Ctrl-C kills the parent shell, so
that this does not print "bye". Different shells behave differently.
> What is definitely required, though, is that the shell not read input or
> alter terminal settings if it is started in the background (possibly
> unless the script explicitly ignored SIGTTOU). This is what the loop
> with tcgetpgrp() does.
Definitely.
Cheers,
Harald van Dijk
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: dash 0.5.11.2, busybox sh 1.32.0, FreeBSD 12.2 sh: spring TTOU but should not i think
2020-12-23 20:18 ` Harald van Dijk
@ 2020-12-24 15:29 ` Jilles Tjoelker
2021-01-10 23:56 ` Harald van Dijk
1 sibling, 0 replies; 13+ messages in thread
From: Jilles Tjoelker @ 2020-12-24 15:29 UTC (permalink / raw)
To: Harald van Dijk; +Cc: Steffen Nurpmeso, DASH shell mailing list, Denys Vlasenko
On Wed, Dec 23, 2020 at 08:18:24PM +0000, Harald van Dijk wrote:
> On 21/12/2020 16:24, Jilles Tjoelker wrote:
> > Also, simply entering the command
> > trap "echo TTOU" TTOU
> > in an interactive shell makes it behave strangely. Created jobs
> > immediately stop, "fg" works once but after that the shell tends to get
> > stuck quickly.
> Good catch, there is some work to be done there too.
> > This seems a good approach, but certain writes to the terminal may need
> > the same treatment, for example the error message when fork fails for
> > the second and further elements of a pipeline. This also makes me wonder
> > why SIGTTOU is ignored at all by default.
> True. This is hard to create a reliable test case for, but there is only
> limited code that can get executed while a job-control-enabled shell may not
> be in the foreground process group.
An rlimit (ulimit -u) will cause fork to fail after a number of
processes, but will not work reliably if other code is executing
concurrently with the same UID.
> If fork fails halfway through a pipeline, it may also be a problem that some
> of the commands in the pipeline may still run.
This can be handled much like the case where an element in the pipeline
fails immediately such as because a utility cannot be found. I am not
sure how well this currently works.
> > In mksh, the issue is resolved slightly differently: setting a trap on
> > TTOU pretends to work but the signal disposition remains set to ignored.
> zsh also rejects traps on TTOU, but does so explicitly:
> zsh: can't trap SIGTTOU in interactive shells
> Amusingly, it prints this in any shell where job control is enabled,
> regardless of whether the shell is interactive.
The rejection makes sense for any shell instance that has job control
and uses tcsetpgrp(), whether interactive or not.
I am not entirely happy with the rejection idea, though, since the check
can be bypassed by temporarily disabling job control:
set +m; trap 'echo TTOU' TTOU; set -m
and running external utilities then fails with:
zsh: can't set tty pgrp: interrupt
> > Tradition is for job control shells to be a process group leader, but I
> > don't really know why. Changing this will not fix the issue entirely
> > anyway since the shell must perform tcsetpgrp() from the background when
> > a foreground job has terminated.
> I've been thinking more about this, and I suspect it's a another conflation
> between interactive mode and job control. In an interactive shell that's not
> executing any external program, you want any Ctrl-C to only send SIGINT to
> that shell, not to any other process. In order for that to work, it needs to
> be its own process group.
> This should, in my opinion, *only* happen for interactive shells, not for
> job-control-enabled non-interactive shells. Consider
> sh -c 'sh -mc "while :; do :; done"; echo bye'
> The behaviour I would want is that Ctrl-C kills the parent shell, so that
> this does not print "bye". Different shells behave differently.
I think the main effect of the -m option is that it places jobs in
separate process groups, which may also be useful in scripts. After
something like:
set -m
foo &
foopid=$!
set +m
it is possible to kill -- "-$foopid" .
Although non-portable kernel features such as cgroups (Linux) or reaper
(FreeBSD) might be more useful for tracking processes like this, the
work to define how to use them in a shell has not been done.
In FreeBSD sh, I extended the possibilities here by allowing job control
without an accessible tty in non-interactive mode. This applies both if
there is no controlling terminal at all and if the shell is in the
background.
In the example
sh -c 'sh -mc "while :; do :; done"; echo bye'
the -m flag seems to have no effect since that shell only runs builtins.
Changing it to
sh -mc 'sh -c "while :; do :; done"; echo bye'
the desired Ctrl+C behaviour can still work because the outer shell
exits when it notices the inner shell has terminated because of SIGINT.
--
Jilles Tjoelker
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] jobs: Block signals during tcsetpgrp
2020-12-19 23:52 ` Harald van Dijk
2020-12-21 16:24 ` Jilles Tjoelker
@ 2021-01-06 4:45 ` Herbert Xu
2021-01-06 21:16 ` Harald van Dijk
1 sibling, 1 reply; 13+ messages in thread
From: Herbert Xu @ 2021-01-06 4:45 UTC (permalink / raw)
To: Harald van Dijk; +Cc: steffen, dash, vda.linux, jilles
Harald van Dijk <harald@gigawatt.nl> wrote:
> On 19/12/2020 22:21, Steffen Nurpmeso wrote:
>> Steffen Nurpmeso wrote in
>> <20201219172838.1B-WB%steffen@sdaoden.eu>:
>> |Long story short, after falsely accusing BSD make of not working
>>
>> After dinner i shortened it a bit more, and attach it again, ok?
>> It is terrible, but now less redundant than before.
>> Sorry for being so terse, that problem crosses my head for about
>> a week, and i was totally mislead and if you bang your head
>> against the wall so many hours bugs or misbehaviours in a handful
>> of other programs is not the expected outcome.
>
> I think a minimal test case is simply
>
> all:
> $(SHELL) -c 'trap "echo TTOU" TTOU; set -m; echo all good'
>
> unless I accidentally oversimplified.
>
> The SIGTTOU is caused by setjobctl's xtcsetpgrp(fd, pgrp) call to make
> its newly started process group the foreground process group when job
> control is enabled, where xtcsetpgrp is a wrapper for tcsetpgrp. (That's
> in dash, the other variants may have some small differences.) tcsetpgrp
> has this little bit in its specification:
>
> Attempts to use tcsetpgrp() from a process which is a member of
> a background process group on a fildes associated with its con‐
> trolling terminal shall cause the process group to be sent a
> SIGTTOU signal. If the calling thread is blocking SIGTTOU sig‐
> nals or the process is ignoring SIGTTOU signals, the process
> shall be allowed to perform the operation, and no signal is
> sent.
>
> Ordinarily, when job control is enabled, SIGTTOU is ignored. However,
> when a trap action is specified for SIGTTOU, the signal is not ignored,
> and there is no blocking in place either, so the tcsetpgrp() call is not
> allowed.
>
> The lowest impact change to make here, the one that otherwise preserves
> the existing shell behaviour, is to block signals before calling
> tcsetpgrp and unblocking them afterwards. This ensures SIGTTOU does not
> get raised here, but also ensures that if SIGTTOU is sent to the shell
> for another reason, there is no window where it gets silently ignored.
>
> Another way to fix this is by not trying to make the shell start a new
> process group, or at least not make it the foreground process group.
> Most other shells appear to not try to do this.
This patch implements the blocking of SIGTTOU (and everything else)
while we call tcsetpgrp.
Reported-by: Steffen Nurpmeso <steffen@sdaoden.eu>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff --git a/src/jobs.c b/src/jobs.c
index 516786f..809f37c 100644
--- a/src/jobs.c
+++ b/src/jobs.c
@@ -1512,7 +1512,13 @@ showpipe(struct job *jp, struct output *out)
STATIC void
xtcsetpgrp(int fd, pid_t pgrp)
{
- if (tcsetpgrp(fd, pgrp))
+ int err;
+
+ sigblockall(NULL);
+ err = tcsetpgrp(fd, pgrp);
+ sigclearmask();
+
+ if (err)
sh_error("Cannot set tty process group (%s)", strerror(errno));
}
#endif
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: dash 0.5.11.2, busybox sh 1.32.0, FreeBSD 12.2 sh: spring TTOU but should not i think
2020-12-21 16:24 ` Jilles Tjoelker
2020-12-21 19:43 ` Steffen Nurpmeso
2020-12-23 20:18 ` Harald van Dijk
@ 2021-01-06 4:46 ` Herbert Xu
2 siblings, 0 replies; 13+ messages in thread
From: Herbert Xu @ 2021-01-06 4:46 UTC (permalink / raw)
To: Jilles Tjoelker; +Cc: harald, steffen, dash, vda.linux
Jilles Tjoelker <jilles@stack.nl> wrote:
>
> This seems a good approach, but certain writes to the terminal may need
> the same treatment, for example the error message when fork fails for
> the second and further elements of a pipeline. This also makes me wonder
> why SIGTTOU is ignored at all by default.
I think the approach of blocking all signals should be able to
resolve this too if anyone cares enough about this case.
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] jobs: Block signals during tcsetpgrp
2021-01-06 4:45 ` [PATCH] jobs: Block signals during tcsetpgrp Herbert Xu
@ 2021-01-06 21:16 ` Harald van Dijk
2021-01-06 22:41 ` Jilles Tjoelker
2021-01-07 7:36 ` Denys Vlasenko
0 siblings, 2 replies; 13+ messages in thread
From: Harald van Dijk @ 2021-01-06 21:16 UTC (permalink / raw)
To: Herbert Xu; +Cc: steffen, dash, vda.linux, jilles
On 06/01/2021 04:45, Herbert Xu wrote:
> This patch implements the blocking of SIGTTOU (and everything else)
> while we call tcsetpgrp.
>
> Reported-by: Steffen Nurpmeso <steffen@sdaoden.eu>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>
> diff --git a/src/jobs.c b/src/jobs.c
> index 516786f..809f37c 100644
> --- a/src/jobs.c
> +++ b/src/jobs.c
> @@ -1512,7 +1512,13 @@ showpipe(struct job *jp, struct output *out)
> STATIC void
> xtcsetpgrp(int fd, pid_t pgrp)
> {
> - if (tcsetpgrp(fd, pgrp))
> + int err;
> +
> + sigblockall(NULL);
> + err = tcsetpgrp(fd, pgrp);
> + sigclearmask();
> +
> + if (err)
> sh_error("Cannot set tty process group (%s)", strerror(errno));
> }
> #endif
While this is a step in the right direction, Jilles has already replied
with an explanation of why this is not enough: if the terminal is in
TOSTOP mode, it's not just tcsetpgrp() that needs to be handled, it's
any write as well that may occur while the shell is not in the
foreground process group. While it may be working according to design
for messages written when the shell is not supposed to be in the
foreground process group, it is another story when the shell is both
responsible for taking itself out of the foreground process group and
for writing a message. This is made worse by the fact that there is no
synchronisation with child processes on errors, so even forcibly
restoring the foreground process group may not be enough: unfortunate
scheduling may result in a child process immediately setting the
foreground process group to the child process after the parent process
attempted to restore it to itself. I have not yet seen a good solution
for this.
Cheers,
Harald van Dijk
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] jobs: Block signals during tcsetpgrp
2021-01-06 21:16 ` Harald van Dijk
@ 2021-01-06 22:41 ` Jilles Tjoelker
2021-01-07 7:36 ` Denys Vlasenko
1 sibling, 0 replies; 13+ messages in thread
From: Jilles Tjoelker @ 2021-01-06 22:41 UTC (permalink / raw)
To: Harald van Dijk; +Cc: Herbert Xu, steffen, dash, vda.linux
On Wed, Jan 06, 2021 at 09:16:58PM +0000, Harald van Dijk wrote:
> On 06/01/2021 04:45, Herbert Xu wrote:
> > This patch implements the blocking of SIGTTOU (and everything else)
> > while we call tcsetpgrp.
> > Reported-by: Steffen Nurpmeso <steffen@sdaoden.eu>
> > Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> > diff --git a/src/jobs.c b/src/jobs.c
> > index 516786f..809f37c 100644
> > --- a/src/jobs.c
> > +++ b/src/jobs.c
> > @@ -1512,7 +1512,13 @@ showpipe(struct job *jp, struct output *out)
> > STATIC void
> > xtcsetpgrp(int fd, pid_t pgrp)
> > {
> > - if (tcsetpgrp(fd, pgrp))
> > + int err;
> > +
> > + sigblockall(NULL);
> > + err = tcsetpgrp(fd, pgrp);
> > + sigclearmask();
> > +
> > + if (err)
> > sh_error("Cannot set tty process group (%s)", strerror(errno));
> > }
> > #endif
> While this is a step in the right direction, Jilles has already replied with
> an explanation of why this is not enough: if the terminal is in TOSTOP mode,
> it's not just tcsetpgrp() that needs to be handled, it's any write as well
> that may occur while the shell is not in the foreground process group. While
> it may be working according to design for messages written when the shell is
> not supposed to be in the foreground process group, it is another story when
> the shell is both responsible for taking itself out of the foreground
> process group and for writing a message. This is made worse by the fact that
> there is no synchronisation with child processes on errors, so even forcibly
> restoring the foreground process group may not be enough: unfortunate
> scheduling may result in a child process immediately setting the foreground
> process group to the child process after the parent process attempted to
> restore it to itself. I have not yet seen a good solution for this.
Comparing this error situation to the normal case, I think the right
solution is to close any stray pipe ends we have, wait for the remaining
child processes and only then report the error (throwing an exception as
normal). The child processes will probably terminate soon because of a
broken pipe, but even if they stop, they will not change the tty
foreground process group any more. The code in jobs.c will then reset
it.
The same error handling applies to the situation where pipe() fails.
This is a bit easier to test reliably, using ulimit -n.
Adding synchronization with the child processes slows down the normal
case for a rare error case, and the synchronization objects such as
pipe, eventfd, SysV semaphore or MAP_SHARED mapping might cause
unexpected issues in strange use cases.
A related bug: if fork fails for a command substitution, the pipe
created for reading the command output remains open (two descriptors).
This one is also in dash as well as FreeBSD sh. Throwing exceptions from
forkshell() may not be the best idea.
--
Jilles Tjoelker
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] jobs: Block signals during tcsetpgrp
2021-01-06 21:16 ` Harald van Dijk
2021-01-06 22:41 ` Jilles Tjoelker
@ 2021-01-07 7:36 ` Denys Vlasenko
1 sibling, 0 replies; 13+ messages in thread
From: Denys Vlasenko @ 2021-01-07 7:36 UTC (permalink / raw)
To: Harald van Dijk
Cc: Herbert Xu, steffen, DASH shell mailing list, Jilles Tjoelker
On Wed, Jan 6, 2021 at 10:17 PM Harald van Dijk <harald@gigawatt.nl> wrote:
> On 06/01/2021 04:45, Herbert Xu wrote:
> > This patch implements the blocking of SIGTTOU (and everything else)
> > while we call tcsetpgrp.
> >
> > Reported-by: Steffen Nurpmeso <steffen@sdaoden.eu>
> > Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> >
> > diff --git a/src/jobs.c b/src/jobs.c
> > index 516786f..809f37c 100644
> > --- a/src/jobs.c
> > +++ b/src/jobs.c
> > @@ -1512,7 +1512,13 @@ showpipe(struct job *jp, struct output *out)
> > STATIC void
> > xtcsetpgrp(int fd, pid_t pgrp)
> > {
> > - if (tcsetpgrp(fd, pgrp))
> > + int err;
> > +
> > + sigblockall(NULL);
> > + err = tcsetpgrp(fd, pgrp);
> > + sigclearmask();
> > +
> > + if (err)
> > sh_error("Cannot set tty process group (%s)", strerror(errno));
> > }
> > #endif
>
> While this is a step in the right direction, Jilles has already replied
> with an explanation of why this is not enough: if the terminal is in
> TOSTOP mode, it's not just tcsetpgrp() that needs to be handled, it's
> any write as well that may occur while the shell is not in the
> foreground process group. While it may be working according to design
> for messages written when the shell is not supposed to be in the
> foreground process group, it is another story when the shell is both
> responsible for taking itself out of the foreground process group and
> for writing a message. This is made worse by the fact that there is no
> synchronisation with child processes on errors, so even forcibly
> restoring the foreground process group may not be enough: unfortunate
> scheduling may result in a child process immediately setting the
> foreground process group to the child process after the parent process
> attempted to restore it to itself. I have not yet seen a good solution
> for this.
How about not simply allowing traps on SIGTTOU, like other shells do?
It's not like there is real-world need to allow usage of this signal
- it's not customarily used by userspace for inter-process signaling,
unlike e.g. SIGTERM or SIGUSR1/2.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: dash 0.5.11.2, busybox sh 1.32.0, FreeBSD 12.2 sh: spring TTOU but should not i think
2020-12-23 20:18 ` Harald van Dijk
2020-12-24 15:29 ` Jilles Tjoelker
@ 2021-01-10 23:56 ` Harald van Dijk
1 sibling, 0 replies; 13+ messages in thread
From: Harald van Dijk @ 2021-01-10 23:56 UTC (permalink / raw)
To: Jilles Tjoelker; +Cc: Steffen Nurpmeso, DASH shell mailing list, Denys Vlasenko
On 23/12/2020 20:18, Harald van Dijk wrote:
> On 21/12/2020 16:24, Jilles Tjoelker wrote:
>> Tradition is for job control shells to be a process group leader, but I
>> don't really know why. Changing this will not fix the issue entirely
>> anyway since the shell must perform tcsetpgrp() from the background when
>> a foreground job has terminated.
> [...]
> This should, in my opinion, *only* happen for interactive shells, not
> for job-control-enabled non-interactive shells. [...]
Consider also an extra difficulty arising from this process group switching:
: | dash -m
When a job-control-enabled shell terminates, or when job control is
disabled via set +m, it attempts to re-join its initial process group
and set that as the foreground process group. This can fail if the
process group has ceased to exist after the shell left it, as it does
here, resulting in:
$ : | dash -m
dash: 0: Cannot set tty process group (No such process)
In theory, because of PID reuse, this may even result in some random
unrelated process group temporarily becoming the foreground process group.
I am leaning towards saying that once the shell has committed to
becoming a process group leader, it should remain one. By basing this on
the shell being interactive rather than on job control being enabled,
this is easier to handle, as as far as POSIX is concerned "interactive"
is a property that cannot be changed once the shell has started: set -i
and set +i are extensions not required by POSIX, and if they are
nonetheless supported it is easy to defend them not being fully
equivalent to specifying or leaving out -i on the shell invocation.
Cheers,
Harald van Dijk
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-01-10 23:57 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-19 17:28 dash 0.5.11.2, busybox sh 1.32.0, FreeBSD 12.2 sh: spring TTOU but should not i think Steffen Nurpmeso
2020-12-19 22:21 ` Steffen Nurpmeso
2020-12-19 23:52 ` Harald van Dijk
2020-12-21 16:24 ` Jilles Tjoelker
2020-12-21 19:43 ` Steffen Nurpmeso
2020-12-23 20:18 ` Harald van Dijk
2020-12-24 15:29 ` Jilles Tjoelker
2021-01-10 23:56 ` Harald van Dijk
2021-01-06 4:46 ` Herbert Xu
2021-01-06 4:45 ` [PATCH] jobs: Block signals during tcsetpgrp Herbert Xu
2021-01-06 21:16 ` Harald van Dijk
2021-01-06 22:41 ` Jilles Tjoelker
2021-01-07 7:36 ` Denys Vlasenko
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).