Change the signal handling strategy to avoid race conditions.

Instead of running the handler directly, the Python signal handler
queues a node in ex.nodes_to_run.  Then the main loop checks this on
every invocation of _Execute().

demo/trap.sh: expose EINTR / Ctrl-C problems with OSH.  OSH traps only
work for basic use cases right now, since we're building on top of
Python's signal module for now.

Addresses issue #60.

core/builtin.py

@@ -142,7 +142,7 @@ def __init__(self):
    names.update(_NORMAL_BUILTINS.keys())
    names.update(_SPECIAL_BUILTINS.keys())
    # TODO: Also complete keywords first for, while, etc.  Bash/zsh/fish/yash
    # all do this.  Also do/done
    # all do this.  See osh/lex/{_KEYWORDS, _MORE_KEYWORDS}.

    self.arg_specs = {}
    self.to_complete = sorted(names)
@@ -937,7 +937,7 @@ def DeclareTypeset(argv, mem, funcs):
      for name in names:
        if name in funcs:
          print(name)
          # TODO: Could print LST, or render LST.  Bash does this.
          # TODO: Could print LST, or render LST.  Bash does this.  'trap' too.
          #print(funcs[name])
        else:
          status = 1
@@ -968,29 +968,23 @@ def DeclareTypeset(argv, mem, funcs):
import signal


class _TrapThunk(object):
class _TrapHandler(object):
  """A function that is called by Python's signal module.

  Similar to process.SubProgramThunk."""

  def __init__(self, ex, node):
    self.ex = ex
  def __init__(self, node, nodes_to_run):
    self.node = node
    self.nodes_to_run = nodes_to_run

  def __call__(self, unused_signalnum, unused_frame):
    """For Python's signal module."""
    # TODO: set -o xtrace/verbose should enable this.
    #log('*** RUNNING TRAP for %d ***', unused_signalnum)
    self.Run()

  def Run(self):
    """For hooks."""
    #log('*** RUNNING TRAP for hook')
    unused_status = self.ex.Execute(self.node)
    #log('*** SETTING TRAP for %d ***', unused_signalnum)
    self.nodes_to_run.append(self.node)

  def __str__(self):
    # Used by trap -p

    # TODO: Abbreviate with fmt.PrettyPrint?
    return str(self.node)

@@ -1033,9 +1027,10 @@ def _GetSignalValue(sig_spec):
# trap -- '' SIGTTIN
# trap -- '' SIGTTOU
#
# CPython registers different default handlers.  Wait until C++ rewrite to
# CPython registers different default handlers.  The C++ rewrite should make
# OVM match sh/bash more closely.

def Trap(argv, traps, ex):
def Trap(argv, traps, nodes_to_run, ex):
  arg, i = TRAP_SPEC.Parse(argv)

  if arg.p:  # Print registered handlers
@@ -1098,13 +1093,13 @@ def Trap(argv, traps, ex):
  if sig_spec in _HOOK_NAMES:
    if sig_spec in ('ERR', 'RETURN', 'DEBUG'):
      util.warn("*** The %r isn't yet implemented in OSH ***", sig_spec)
    traps[sig_spec] = _TrapThunk(ex, node)
    traps[sig_spec] = _TrapHandler(node, nodes_to_run)
    return 0

  # Register a signal.
  sig_val = _GetSignalValue(sig_spec)
  if sig_val is not None:
    handler = _TrapThunk(ex, node)
    handler = _TrapHandler(node, nodes_to_run)
    # For signal handlers, the traps dictionary is used only for debugging.
    traps[sig_spec] = handler

core/cmd_exec.py

@@ -137,6 +137,7 @@ def __init__(self, mem, fd_state, status_lines, funcs, completion,
    self.bool_ev = expr_eval.BoolEvaluator(mem, exec_opts, self.word_ev)

    self.traps = {}  # signal/hook name -> callable
    self.nodes_to_run = []  # list of nodes, appended to by signal handlers
    self.dir_stack = state.DirStack()

    # TODO: Pass these in from main()
@@ -323,7 +324,7 @@ def _RunBuiltin(self, builtin_id, argv):
      status = self._Source(argv)

    elif builtin_id == EBuiltin.TRAP:
      status = builtin.Trap(argv, self.traps, self)
      status = builtin.Trap(argv, self.traps, self.nodes_to_run, self)

    elif builtin_id == EBuiltin.UMASK:
      status = builtin.Umask(argv)
@@ -1100,17 +1101,27 @@ def _Dispatch(self, node, fork_external):
    return status, check_errexit

  def _Execute(self, node, fork_external=True):
    """Does redirects, calls _Dispatch(), and does errexit check.
    """Apply redirects, call _Dispatch(), and performs the errexit check.

    Args:
      node: of type AstNode
      node: ast.command
      fork_external: if we get a SimpleCommand that is an external command,
        should we fork first?  This is disabled in the context of a pipeline
        process and a subshell.
    """
    # No redirects to evaluate.
    # NOTE: Function definitions have redirects, but we do NOT want to evaluate
    # redirects them yet!  They are evaluated on every invocation instead!
    # See core/builtin.py for the Python signal handler that appends to this
    # list.

    if self.nodes_to_run:
      # Make a copy and clear it so we don't cause an infinite loop.
      to_run = list(self.nodes_to_run)
      del self.nodes_to_run[:]
      for node in to_run:
        self._Execute(node)

    # These nodes have no redirects.  NOTE: Function definitions have
    # redirects, but we do NOT want to evaluate them yet!  They're evaluated
    # on every invocation.
    if node.tag in (
        command_e.NoOp, command_e.Assignment, command_e.ControlFlow,
        command_e.Pipeline, command_e.AndOr, command_e.CommandList,
@@ -1212,10 +1223,10 @@ def ExecuteAndRunExitTrap(self, node):
    #import traceback
    #traceback.print_stack()

    # NOTE: The trap itself can call exit!
    thunk = self.traps.get('EXIT')
    if thunk:
      thunk.Run()
    # NOTE: The trap handler itself can call exit!
    handler = self.traps.get('EXIT')
    if handler:
      self.Execute(handler.node)

    return status

demo/trap.sh

@@ -56,4 +56,57 @@ start-and-kill() {
  echo status=$?
}

num_signals=0

ignore-n-times() {
  (( num_signals++ ))

  if [[ $num_signals -le 2 ]]; then
    echo "Received signal $num_signals -- IGNORING"
  else
    echo "Removing this signal handler; next one will be the default"
    trap - TERM
  fi
}

# In bash: Run this and hit Ctrl-C four times to see the handler in action!
#
# NOTE: Ctrl-C doesn't work in Python because Python does stuff with SIGINT!
# We could disable KeyboardInterrupt entirely in the OVM build?  But still need
# the signal module!

sleep-and-ignore() {
  trap ignore-n-times TERM
  for i in $(seq 10); do
    echo $i
    sleep 0.2
  done
}

# NOTE: osh has EINTR problems here!
#
# File "/home/andy/git/oilshell/oil/bin/../core/process.py", line 440, in WaitUntilDone
# if not waiter.Wait():
# File "/home/andy/git/oilshell/oil/bin/../core/process.py", line 632, in Wait
#   pid, status = os.wait()
# OSError: [Errno 4] Interrupted system call

kill-sleep-and-ignore() {
  sleep-and-ignore &
  local last_pid=$!

  echo "Started $last_pid"

  # Hm sometimes the signal gets ignored?  You can't just kill it 3 times?
  for i in $(seq 5); do
    kill -s SIGTERM $last_pid
    echo kill status=$?
    sleep 0.1
  done

  wait
  echo wait status=$?
}

"$@"