bug-coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#8938: make timeout and CTRL-C


From: Pádraig Brady
Subject: bug#8938: make timeout and CTRL-C
Date: Tue, 28 Jun 2011 19:49:47 +0100
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.8) Gecko/20100227 Thunderbird/3.0.3

On 27/06/11 23:55, Alan Curry wrote:
> =?UTF-8?Q?P=C3=A1draig?= Brady writes:
>>
>> This is a multi-part message in MIME format.
>> --------------000003030307000505070101
>> Content-Type: text/plain; charset=ISO-8859-1
>> Content-Transfer-Encoding: 8bit
>>
>> On 27/06/11 21:12, Alan Curry wrote:
>>>
>>> What seems to be happening is that make *doesn't* create a process group,
>>> therefore assumes that when it gets a SIGINT, its children have already
>>> gotten it too, and it just waits for them to die. A child that puts itself
>>> into a new process group screws this up (as would kill -2 `pidof make`).
>>
>> Thanks for the analysis Alan.
>> Yes you're right I think.
>> In any case the important point is that timeout sets itself as group leader,
>> and is not the foreground group.
> 
> Right, we have a tree of process groups that goes roughly
> shell->make->timeout and the one in the middle of the tree is the
> foreground, receiving tty-based signals.
> 
>>
>>>
>>> I think the answer is that timeout should put itself into the foreground.
>>> That way it would get the SIGINT. make wouldn't get it, but wouldn't need 
>>> to.
>>> timeout would exit quickly after SIGINT and make would proceed or abort
>>> according to the exit code.
>>
>> I've a version locally here actually that calls tcsetpgrp() but I discounted
>> that as it's not timeout's place to call that I think.
>> timeout sets itself as group leader so that it can kill everything it starts,
>> but it shouldn't need to grab the foreground group as the shell (or make)
>> may be starting it in the background etc.
> 
> It seems like this is a misuse of process groups, using them as if they
> were a handle for killing a whole tree of processes. That's not what
> they're for. Process groups were invented to support job control, which
> means the only program that was supposed to mess with them was csh. Only
> the lack of a "kill process tree" primitive (and the fact that you can't
> even query the process tree easily) tempts us into using process groups
> as a shortcut.
> 
> Any non-job-control-aware parent process will have a problem with
> timeout's behavior. We've already seen what GNU make does. pmake simply
> dies of the SIGINT and leaves the child processes lingering (it probably
> also assumes they got the SIGINT, and doesn't bother waiting for them).
> 
> In an interactive shell with job control disabled (set +m in most
> Bourne-ish shells), the behavior is not good there either. dash, bash,
> and posh all act like GNU make, appearing to ignore the SIGINT. zsh acts
> more like pmake, printing a new prompt but leaving the timeout and its
> child running.
> 
> timeout's pgrp behavior only appears harmless when the parent process is
> a shell with job control, which expects its children to be in separate
> process groups. But in that case, timeout doesn't need to put itself in
> a new process group because the shell has already done so.
> 
> So I suggest that if you create a process group, you take on the
> responsibility of behaving like a job control shell in other ways,
> including managing the foreground group. (An important piece of that is
> remembering the original value and restoring it before you exit).

I'm still not convinced we need to be messing with tcsetpgrp()
but you're right in that the disconnect between the timeout
process group and that of whatever starts `timeout` should be bridged.

I'm testing the attached patch at the moment (which I'll split into 2).
It only creates a separate group for the child that `timeout` execs,
leaving the timeout process in the original group to propagate signals down.

I'll need to do lots of testing with this before I commit.

cheers,
Pádraig.

Attachment: timeout-groups.diff
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]