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

[Linus Torvalds]

On Fri, Feb 11, 2011 at 1:30 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> The other case is that the child process was quick and already exited.
> You get ^C, but the child never did. When you do the waitpid(), you'll
> never get the EINTR, because there was no actual wait.

Ok, so here's a suggested patch. It took much longer than expected,
because I found another independent race while trying to fix this:
bash did the "restore_sigint_handler()" _after_ having already tested
the "wait_sigint_received" value, so what could happen is that the
SIGINT would happen _after_ the test of that flag, but before the
signal handler was restored, so bash would (once more) drop the ^C on
the floor.

So there are two fixes here, and while I have by no means done
exhaustive testing, this patch seems to make the problem with dropped
^C really really hard for me to trigger.

I do believe it's still racy, it's just that I think it is much less
racy than it used to be.

And maybe I introduced some new bug. There may be a reason why the
sigint-handler was restored too late. But it not only passes my ^C
tests, it seems to pass "make test" too, so it can't be horribly
broken.

And bash _was_ horribly broken wrt ^C before.

I tried to make the "child_blocked_sigint" logic very trivial and
obvious, and I added comments for it. But if there are any questions
about the patch, feel free to holler.

The basic rule is:
 - we only think that a child blocked sigint if we actually get EINTR
from a system call (and the signal handler set wait_sigint_received)
 - if the child returns with WSIGNALLED and WTERMSIG()==SIGINT, then
we say "it clearly didn't block it after all, or it emulated dying
with SIGINT, so we clear that previous heuristic".
 - then, instead of checking how the child died, we just use the new
'child_blocked_sigint' flag instead.

So instead of testing

  wait_sigint_received && WIFSIGNALED (s) && WTERMSIG (s) == SIGINT

it now tests

  wait_sigint_received && !child_blocked_sigint

which seems to match the comments better anyway.

Long story short: maybe the patch is buggy. But the issue is real, and
the patch looks largely sensible to me.

                              Linus

patch.diff
Description: Text Data