httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jim Jagielski <...@jaguNET.com>
Subject Re: svn commit: r1778319 - /httpd/httpd/trunk/modules/core/mod_watchdog.c
Date Wed, 11 Jan 2017 17:17:26 GMT

> On Jan 11, 2017, at 12:12 PM, Joe Orton <jorton@redhat.com> wrote:
> 
> On Wed, Jan 11, 2017 at 11:08:29AM -0500, Jim Jagielski wrote:
>> This is to address the following bug:
>> 
>>  https://bugzilla.redhat.com/show_bug.cgi?id=1410883
> 
> Thanks a lot Jim!
> 
>> The only reason why I can see why the orig idea to use s->process->pool
>> was to make watchdog run independent of any restarts of httpd
>> itself... that is, a truly independent watchdog. But that would
>> imply that you can't reconfig watchdog on restarts which
>> doesn't seem quite right...
> 
> Since the callbacks registered with the watchdog come from modules, and 
> modules have lifetime of pconf, it seems right to me to use pconf.  (I 
> would guess that without this fix, if at restart you unloaded a module 
> which had registered a watchdog, it would segfault the server?)
> 
> At restart time when we re-enter the main() loop and clear pconf, 
> that'll run the pre_cleanup wd_worker_cleanup() for each registered 
> worker.  That invokes apr_thread_join() which waits for the worker 
> thread to terminate.   I guess that is sufficient because each thread 
> will then query the MPM state, see "AP_MPMQ_STOPPING", and terminate.
> 
> Not very confident in that though.
> 

Doing some more testing, I can't see offhand any "reasonable"
way to use s->process->pool as the parent pool without a
lot of nasty, ugly hacks that ignore any but the 2nd (re)start,
etc... which would cause us to leak memory in any case.

So I think the fix is *the* fix.

Will propose for backport in a bit.

Mime
View raw message