Re: [BUG] Bash not reacting to Ctrl-C

[Chet Ramey]

> So I don't think my patch is really doing what it _intends_ to do.

Let's take a step back and approach this a different way.  Instead of
trying to intuit whether or not the child did anything with the SIGINT,
let's try to make the race condition smaller.

The following patch is a very small change to jobs.c that makes
wait_sigint_handler only pay attention and set wait_sigint_received when
the shell is actually in waitpid() waiting for the child.  It uses a
semaphore around the call to waitpid to effect that, with a little
bookkeeping and cleanup code.  When the shell gets a SIGINT while not
actually waiting for a child, it restores the old handler and sends
SIGINT to itself.  Since wait_signal_handler isn't installed if the
existing handler is SIG_IGN, this should do the right thing.  Please test
it and let me know what effect it has.

Chet

*** ../bash-4.2-patched/jobs.c  2011-01-07 10:59:29.000000000 -0500
--- jobs.c      2011-03-07 10:40:36.000000000 -0500
***************
*** 2212,2217 ****
--- 2212,2220 ----
  #define INVALID_SIGNAL_HANDLER (SigHandler *)wait_for_background_pids
  static SigHandler *old_sigint_handler = INVALID_SIGNAL_HANDLER;
  
+ static int wait_sigint_received;
+ static int waiting_for_child;
+ 
  static void
  restore_sigint_handler ()
  {
***************
*** 2219,2229 ****
      {
        set_signal_handler (SIGINT, old_sigint_handler);
        old_sigint_handler = INVALID_SIGNAL_HANDLER;
      }
  }
  
- static int wait_sigint_received;
- 
  /* Handle SIGINT while we are waiting for children in a script to exit.
     The `wait' builtin should be interruptible, but all others should be
     effectively ignored (i.e. not cause the shell to exit). */
--- 2222,2231 ----
      {
        set_signal_handler (SIGINT, old_sigint_handler);
        old_sigint_handler = INVALID_SIGNAL_HANDLER;
+       waiting_for_child = 0;
      }
  }
  
  /* Handle SIGINT while we are waiting for children in a script to exit.
     The `wait' builtin should be interruptible, but all others should be
     effectively ignored (i.e. not cause the shell to exit). */
***************
*** 2256,2262 ****
  
    /* XXX - should this be interrupt_state?  If it is, the shell will act
       as if it got the SIGINT interrupt. */
!   wait_sigint_received = 1;
  
    /* Otherwise effectively ignore the SIGINT and allow the running job to
       be killed. */
--- 2258,2271 ----
  
    /* XXX - should this be interrupt_state?  If it is, the shell will act
       as if it got the SIGINT interrupt. */
!   if (waiting_for_child)
!     wait_sigint_received = 1;
!   else
!     {
!       last_command_exit_value = 128+SIGINT;
!       restore_sigint_handler ();
!       kill (getpid (), SIGINT);
!     }
  
    /* Otherwise effectively ignore the SIGINT and allow the running job to
       be killed. */
***************
*** 2396,2401 ****
--- 2405,2411 ----
    if (job_control == 0 || (subshell_environment&SUBSHELL_COMSUB))
      {
        old_sigint_handler = set_signal_handler (SIGINT, wait_sigint_handler);
+       waiting_for_child = 0;
        if (old_sigint_handler == SIG_IGN)
        set_signal_handler (SIGINT, old_sigint_handler);
      }
***************
*** 2524,2529 ****
--- 2534,2542 ----
        give_terminal_to (shell_pgrp, 0);
      }
  
+   /* Restore the original SIGINT signal handler before we return. */
+   restore_sigint_handler ();
+ 
    /* If the command did not exit cleanly, or the job is just
       being stopped, then reset the tty state back to what it
       was before this command.  Reset the tty state and notify
***************
*** 2589,2595 ****
          if (WIFSIGNALED (s) && WTERMSIG (s) == SIGINT && signal_is_trapped 
(SIGINT) == 0)
            {
              UNBLOCK_CHILD (oset);
-             restore_sigint_handler ();
              old_sigint_handler = set_signal_handler (SIGINT, SIG_DFL);
              if (old_sigint_handler == SIG_IGN)
                restore_sigint_handler ();
--- 2602,2607 ----
***************
*** 2615,2623 ****
  
    UNBLOCK_CHILD (oset);
  
-   /* Restore the original SIGINT signal handler before we return. */
-   restore_sigint_handler ();
- 
    return (termination_state);
  }
  
--- 2627,2632 ----
***************
*** 3090,3096 ****
--- 3099,3107 ----
          waitpid_flags |= WNOHANG;
        }
  
+       waiting_for_child++;
        pid = WAITPID (-1, &status, waitpid_flags);
+       waiting_for_child--;
  
        /* WCONTINUED may be rejected by waitpid as invalid even when defined */
        if (wcontinued && pid < 0 && errno == EINVAL)

-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
                 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, ITS, CWRU    chet@case.edu    http://cnswww.cns.cwru.edu/~chet/