Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Received-SPF: neutral (athena.apache.org: local policy)
Message-ID: <498FD47F.8070003@apache.org>
Date: Mon, 09 Feb 2009 08:00:15 +0100
From: Mladen Turk <mturk@apache.org>
User-Agent: Thunderbird 2.0.0.17 (X11/20080915)
MIME-Version: 1.0
To: dev@httpd.apache.org
Subject: Re: svn commit: r741947 - in /httpd/httpd/trunk/modules/mappers:
 config9.m4 mod_watchdog.c mod_watchdog.h
References: <20090207195153.5B43C238896B@eris.apache.org>
 <498EBEAB.7000600@apache.org>
In-Reply-To: <498EBEAB.7000600@apache.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

Ruediger Pluem wrote:
> 
>> +        }
>> +        if (w->is_running && w->callbacks == NULL) {
>> +            /* This is hook mode watchdog
>> +             * running on WatchogInterval
>> +             */
>> +            w->step += (apr_time_now() - curr);
>> +            if (w->step >= wd_interval) {
>> +                if (!ctx)
>> +                    apr_pool_create(&ctx, w->pool);
> 
> Is this check really needed? ctx should be always NULL here.
> 

Yes, it's NULL, but only for the first callback.
If there are multiple callbacks they will all use
the same pool for this call.
Also, pool gets created only if one of the callbacks
actually times out.

>> +
>> +    /* This mutex fixes problems with a fast start/fast end, where the pool
>> +     * cleanup was being invoked before the thread completely spawned.
>> +     */
> 
> Can you elaborate this a bit more (maybe with an example)?
> I currently do not understand which problem is solved here.
>

Well, I reused the mod_heartbeat code here, and I think it's
very useful to ensure the thread actually gets created and
running before we allow to kill the child.

>> +    apr_thread_mutex_lock(w->startup);
>> +
>> +    /* Start the newly created watchdog */
>> +    rc = apr_thread_create(&w->thread, NULL, wd_worker, w, p);
>> +    if (rc) {
>> +        apr_pool_cleanup_kill(p, w, wd_worker_cleanup);
> 
> I don't see that this cleanup gets registered anywhere.
>

That got slipped during copy/paste from my dev branch.
Fixed, thanks.


Regards
-- 
^(TM)