httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeff Trawick <traw...@gmail.com>
Subject Re: [mod_fcgid patch] reap children without the zombie scan
Date Sat, 03 Oct 2009 18:44:02 GMT
On Thu, Oct 1, 2009 at 8:01 PM, Jeff Trawick <trawick@gmail.com> wrote:

> (just fixing subject)
>
> On Thu, Oct 1, 2009 at 7:55 PM, Ricardo Cantu <ricardo@smartcsc.com>wrote:
>
>> On Tuesday 29 September 2009 4:20:49 pm you wrote:
>> > On Tue, Sep 29, 2009 at 4:59 PM, Ricardo Cantu <ricardo@smartcsc.com>
>> wrote:
>> > > On Tuesday 29 September 2009 2:31:21 pm Jeff Trawick wrote:
>> > > > ZombieScanInterval (leave alone until processes can be reaped
>> > >
>> > > differently)
>> > > Working on a patch for this one. Don't want to duplicate work, so let
>> me
>> > > know
>> > > if anybody else is working on this.
>> >
>> > not me
>> >
>> > I hope that, for Unix, processes can be reaped as with the MPMs: instead
>> of
>> > asking if a specific pid has exited (for each pid in the list), ask if
>> any
>> > pid has exited and if so find it in the list and handle.
>> >
>>
>> Well, here it is. My patch to reap the children when they exit rather than
>> check the list for zombies. Before I take out the old logic for the zombie
>> scan I would like to hear some input on the code.
>>
>> basically,
>>
>> apr_proc_other_child_register()  - to register a callback when child
>> exits.
>>
>> sigaction(SIGCHLD, &sa, NULL) - to listen for children dying.
>>
>> apr_proc_other_child_refresh_all(APR_OC_REASON_RESTART) - called when
>> SIGCHLD
>> received so callback will be called on the correct registered child.
>>
>> fcgid_child_maint - The callback. Cleans up the various lists and prints
>> out
>> log info.
>>
>> And that's it.
>>
>>
>
Is this right?

before:

when process mgr has awakened (every second) and ZombieScanInterval has
elapsed:

lock table
foreach table entry (process spawned by process mgr) {
    call waitpid on pid and see if it has exited
    if exited, move to free list
}
unlock table

and this was done whether or not a child exited

now:

when awakened by SIGCHILD, in signal handler:

foreach registered other child (process spawned by process mgr) {
    call waitpid on pid and see if it has exited
    if exited {
        lock table
        search table for child and move to free list
        unlock table
    }
}

If so:  I don't see that using the other child API has bought us much.  At
least we don't lock the table unless a process has actually exited.

If the process mgr calls waitpid with a wildcard to see if any pid has
exited, then at least a list doesn't have to be searched unless there is
work to do.  The prefork MPM does that, then calls
apr_proc_other_child_alert() to see if it is for an other-child.  (The alert
function then runs the list of other children to find the pid.)

SIGCHLD can be used to unblock from waiting for (command, 1 second) sooner*,
but calling interesting code from the SIGCHLD handler should be avoided if
at all possible.  (Windows wouldn't have that flow of control either.)

*I dunno if the mpm_common function currently used will return on EINTR

Thoughts?

(BTW, it is easier on reviewers if one patch tries to solve exactly one
problem.)

Mime
View raw message