On 3/11/11 1:45 AM, Ingo Molnar wrote:
>
> * Chet Ramey <[email protected]> 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 [email protected] 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)
{
