John Levon wrote:

> On Mon, Mar 22, 2004 at 10:00:22PM +0000, Angus Leeming wrote:
> 
>> > We only need to reap children every 10 minutes or whatever
>> 
>> Then we go back to the timer and the whole thing has been a waste
>> of time?
> 
> Why do you say that? It's completely normal and expected practice.
> 
>> It is trivially easy to execute code from the signal handler. Maybe
>> that's OK in this case? The main code is not going to be in a
>> fragile state because SIGCHLD is not an error. I guess that we
>> still need
> 
> It's still asynchronous. 

I don't think you've read my proposed code. I'm pretty sure it's 
secure in the face of asynchronicity.

> The difference from the current code is
> that we know there's likely to be a child waiting to be reaped. 

Agreed.

> We drop out of the Qt loop to check the sigatomic_t. 
> If it's set, then we go and reap the child, secure that we're
> not somewhere in the forked controller code.

That was proposal 2 on which you commented: "Excuse me if I'm 
misunderstanding, is this going to busy loop when we've
no events on the queue ?"

> Because everything runs from the Qt event handler, we know we're
> safe.

Of course its safe, but its ridiculous.

> Of course, you still have to be careful:
> 
> reapTimer()
> {
> while (sigatomic_value) {
> sigatomic_value = 0;
> reapAnyAndAllChildren();
> }
> }
> 
> or some such. You probably need a 'volatile' keyword on the
> sigatomic too.

Your just repeating the code I wrote (Proposal 2). Whilst it is good 
of you to comment on my suggestions, I think you might first read the 
code. Saves all sorts of confusion and frustration.

Incidentally, assignments to integer types the size of int or smaller, 
or to pointers, are atomic. So it's perfectly OK to use bool here.

> I don't know why the current code loops across all children. We
> should just repeatedly wait for any child, and reap as needed.

Agreed, but that's a totally separate issue.

>> void child_handler(int)
>> {
>>     while (process_signal) {
>>         process_signal = false;
>>         ForkedcallController::get().handleCompletedProcesses();
> 
> Racy. If we're in the forked controller code and we take a signal,
> we end up calling back into the forked controller code. Suddenly the
> controller code needs sigprocmask() all over the place or we run the
> risk of racing against ourselves in signal context.

It was protected by "processing_signal". It's impossible for the 
second signal to get back into the forked controller code.

> regards,
> john

-- 
Angus

Reply via email to