Colm: >> 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. Chris. -- GPG Key ID: 366A375B GPG Key Fingerprint: 485E 5041 17E1 E2BB C263 E4DE C8E3 FA36 366A 375B
