httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chris Darroch <>
Subject Re: [PATCH] #39275 MaxClients on startup [Was: Bug in 2.0.56-dev]
Date Tue, 18 Apr 2006 01:57:30 GMT

>>    The worker and event MPMs would use these to track their
>> non-worker threads; and the parent process for these MPMs could
>> monitor them as per option C to decide when the child process's
>> workers were ready to be counted.
> +1, I think this could be very useful, I came accross the same kind of
> problems when looking at handling the odd mod_cgid race conditions when
> doing graceful stops, but in the end didn't solve them (we still have
> slightly different death semantics between prefork and worker-like MPM's
> for cgi termination).

   Great, I'll get started on this first thing tomorrow.  I'm
away Thurs-Mon this week but with luck will have some patches
before I go for all these co-mingled issues.

>>    Finally, a question without checking the code first ... I notice
>> that worker.c has the following, taken from prefork.c:
>>     /* fork didn't succeed. Fix the scoreboard or else
>>      * it will say SERVER_STARTING forever and ever
>>      */
>>     ap_update_child_status_from_indexes(slot, 0, SERVER_DEAD, NULL);
>> Is that right?  Should it perhaps cycle through and set all
>> non-DEAD workers to DEAD?  I'm thinking that in prefork,
>> threads_limit is set to 1, so mod_status only checks the first
>> thread in a slot, but here it'll check all of them.
> I'm not sure what you mean. As in, all potential workers globally? That
> child isn't going to have any actual workers, since child_main() never
> got run.

   What I noticed is that the prefork MPM seems to consistently
use the first worker_score in the scoreboard for all its record-
keeping; that makes sense because its thread_limit is always 1.
So in make_child(), it sets that worker_score to SERVER_STARTING,
does the fork(), and if that fails, resets the worker_score
to SERVER_DEAD.  No problems there.

   However, the code seems to have been copied over into the
worker MPM, and that particular if-fork()-fails bit still does
the same business, even though other things have changed around
it.  In particular, multiple worker_score structures are in
use for each child (ap_threads_per_child of them), and
make_child() doesn't begin by setting them all to SERVER_STARTING --
or even one of them -- so having it subsequently handle the
fork()-fails error condition by resetting the first one to
SERVER_DEAD seems incorrect.

   Further, make_child() may be creating a child to replace
a gracefully exiting one, and if so, it shouldn't actually
touch any of the worker_score statuses.  That's because what
start_threads() in the child does -- in the case where fork()
succeeds -- is to watch for threads in the old process that
have marked their status as SERVER_GRACEFUL and only then
initiate new workers to replace them.  So in fact, if fork()
fails, such old workers may still be executing and their
status values may not yet be SERVER_GRACEFUL.  As things stand,
if one of them happens to be have a scoreboard index of 0, its
value will get overwritten by make_child().

   I think probably the right thing is for make_child() just
to do nothing here.  That makes the same assumption the rest
of the code does, namely, that worker threads will reach their
normal termination and set their statuses to SERVER_DEAD or
SERVER_GRACEFUL; and if a child process does fail, then
the parent process's will notice in server_main_loop() and
set all of the child's threads' statuses to SERVER_DEAD.

   Anyway, I'll include that change in my patchset for review;
I may of course have missed something important.


GPG Key ID: 366A375B
GPG Key Fingerprint: 485E 5041 17E1 E2BB C263  E4DE C8E3 FA36 366A 375B

View raw message