On 3/11/11 1:45 AM, Ingo Molnar wrote:
> 
> * Chet Ramey <chet.ra...@case.edu> wrote:
> 
>>> So the above trace is one that my patch would have handled correctly
>>> (since it has no EINTR). Maybe a combination of the two approaches
>>> would work even better?
>>
>> I installed what is essentially the union of your changes and mine.  Thanks
>> for everyone's help with this.
> 
> Thanks! Mind attaching the final patch to this thread, so that people can try 
> out 
> and test the patch you mean?
Sure, attached.

Chet
-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
                 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, ITS, CWRU    c...@case.edu    http://cnswww.cns.cwru.edu/~chet/
*** ../bash-4.2-patched/jobs.c  2011-01-07 10:59:29.000000000 -0500
--- jobs.c      2011-03-11 10:57:57.000000000 -0500
***************
*** 2213,2216 ****
--- 2213,2220 ----
  static SigHandler *old_sigint_handler = INVALID_SIGNAL_HANDLER;
  
+ static int wait_sigint_received;
+ static int child_caught_sigint;
+ static int waiting_for_child;
+ 
  static void
  restore_sigint_handler ()
***************
*** 2220,2228 ****
        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
--- 2224,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
***************
*** 2257,2261 ****
    /* 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
--- 2260,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
***************
*** 2393,2400 ****
  
    /* This is possibly a race condition -- should it go in stop_pipeline? */
!   wait_sigint_received = 0;
    if (job_control == 0 || (subshell_environment&SUBSHELL_COMSUB))
      {
        old_sigint_handler = set_signal_handler (SIGINT, wait_sigint_handler);
        if (old_sigint_handler == SIG_IGN)
        set_signal_handler (SIGINT, old_sigint_handler);
--- 2403,2411 ----
  
    /* This is possibly a race condition -- should it go in stop_pipeline? */
!   wait_sigint_received = child_caught_sigint = 0;
    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);
***************
*** 2448,2452 ****
--- 2459,2465 ----
  #  endif
          queue_sigchld = 1;
+         waiting_for_child++;
          r = waitchld (pid, 1);
+         waiting_for_child--;
  #  if defined (MUST_UNBLOCK_CHLD)
          sigaction (SIGCHLD, &oact, (struct sigaction *)NULL);
***************
*** 2490,2493 ****
--- 2503,2509 ----
    while (PRUNNING (child) || (job != NO_JOB && RUNNING (job)));
  
+   /* Restore the original SIGINT signal handler before we return. */
+   restore_sigint_handler ();
+ 
    /* The exit state of the command is either the termination state of the
       child, or the termination state of the job.  If a job, the status
***************
*** 2586,2594 ****
             parent. */
          s = job_signal_status (job);
!       
!         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)
--- 2602,2609 ----
             parent. */
          s = job_signal_status (job);
! 
!         if (child_caught_sigint == 0 && signal_is_trapped (SIGINT) == 0)
            {
              UNBLOCK_CHILD (oset);
              old_sigint_handler = set_signal_handler (SIGINT, SIG_DFL);
              if (old_sigint_handler == SIG_IGN)
***************
*** 2616,2622 ****
    UNBLOCK_CHILD (oset);
  
-   /* Restore the original SIGINT signal handler before we return. */
-   restore_sigint_handler ();
- 
    return (termination_state);
  }
--- 2631,2634 ----
***************
*** 3118,3124 ****
--- 3130,3150 ----
         the only other error POSIX says it can return is EINTR. */
        CHECK_TERMSIG;
+ 
+       /* If waitpid returns -1/EINTR and the shell saw a SIGINT, then we
+        assume the child has blocked or handled SIGINT.  In that case, we
+        require the child to actually die due to SIGINT to act on the
+        SIGINT we received; otherwise we assume the child handled it and
+        let it go. */
+       if (pid < 0 && errno == EINTR && wait_sigint_received)
+       child_caught_sigint = 1;
+ 
        if (pid <= 0)
        continue;       /* jumps right to the test */
  
+       /* If the child process did die due to SIGINT, forget our assumption
+        that it caught or otherwise handled it. */
+       if (WIFSIGNALED (status) && WTERMSIG (status) == SIGINT)
+         child_caught_sigint = 0;
+ 
        /* children_exited is used to run traps on SIGCHLD.  We don't want to
           run the trap if a process is just being continued. */
***************
*** 3308,3312 ****
         otherwise act as if we got the interrupt. */
        if (wait_sigint_received && interactive_shell == 0 &&
!         WIFSIGNALED (child->status) == 0 && IS_FOREGROUND (job) &&
          signal_is_trapped (SIGINT))
        {
--- 3334,3338 ----
         otherwise act as if we got the interrupt. */
        if (wait_sigint_received && interactive_shell == 0 &&
!         child_caught_sigint && IS_FOREGROUND (job) &&
          signal_is_trapped (SIGINT))
        {
***************
*** 3330,3334 ****
         process by another process (or itself).  If the shell did receive the
         SIGINT, it needs to perform normal SIGINT processing. */
!       else if (wait_sigint_received && (WTERMSIG (child->status) == SIGINT) &&
              IS_FOREGROUND (job) && IS_JOBCONTROL (job) == 0)
        {
--- 3356,3361 ----
         process by another process (or itself).  If the shell did receive the
         SIGINT, it needs to perform normal SIGINT processing. */
!       else if (wait_sigint_received &&
!             child_caught_sigint == 0 &&
              IS_FOREGROUND (job) && IS_JOBCONTROL (job) == 0)
        {

Reply via email to