dash.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* 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	[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).