-1. The problems detailed in the messages are a misconfigured server, not
a bug in the code. The threaded MPM should count ALL idle threads,
whether they are a part of the process that was just told to die or not.
Any idle threads that are a part of the process that was just told to die
will be dead the next time idle_server_maintenance is run, so everything
works out well in the end.
You must count the non-idle threads of the server that was told to die,
because if you don't you take the real chance of going over MaxClients.
Also, the process_status field is going away soon. It isn't used
anywhere, and it isn't needed.
Ryan
On Sat, 14 Jul 2001, GUMMALAM,MOHAN (HP-Cupertino,ex2) wrote:
> I propose the following patch [PATCH A]: It will partially fix the unwanted
> child deaths problem (symptoms mentioned in the mails included below). It
> fixes the problem by making sure that perform_idle_server_maintenance() does
> not count the threads of the process that recd the POD, in the calculation
> of idle_thread_count. To do that I have used
> ap_scoreboard_image->parent[process_slot]->process_status field. I am
> temporarily using an already defined value, SB_IDLE_DIE. If the general
> idea is acceptable, I can work on solidifying the details. PATCH A is
> attached below.
>
> However this patch exposes another problem in the code - by this new fix,
> although the untargetted childs do not get a POD, the targetted child
> process does not die immediately either. Here is why that happens: In the
> worker_thread() routine in threaded.c, at the instant when worker 1.0
> (represented in the <process_slot>.<thread_slot> format, i.e, 1 is the
> process_slot and 0 is the thread_slot) gets the POD, the remaining threads
> of process 1 are all waiting at the apr_lock_acquire(accept_mutex). If the
> web-server is really idle, the chances are slim that all the remaining
> worker threads for process 1 would acquire the lock in a very short time.
> As an effect, the remaining worker threads of process 1 do not die
> immediately. To resolve this:
>
> SOLUTION 1: I plan to temporarily implement a new
> apr_lock_acquire_timeout() function, which would cause the threads waiting
> on the mutex to give-up after sometime. The piece of code would look
> something like this:
>
> while ((rv = SAFE_ACCEPT(apr_lock_acquire_timeout(accept_mutex, timevalue)))
> != APR_SUCCESS) {
> if (check_if_timer_popped)
> if (workers_may_exit)
> break;
> else { /* apr_lock_acquire failed */
> ap_log_error(....);
> workers_may_exit = 1;
> }
> }
>
> I know that this would cause some performance impact, but my guess is that
> it would not be a lot, especially if we keep the timevalue reasonably high.
> However, in order to get the functionality of
> perform_idle_server_maintenance() working right, we _will_ have to implement
> the above solution (or maybe something similar)!
>
> SOLUTION 2: One could use a slightly more _involved_ approach, where the
> dying thread could send a signal to its sibling threads, each of which will
> then handle that signal with a graceful exit.
>
> SOLUTION 3: We could turn our heads the other way by not worrying about this
> situation at all, since these threads would eventually die (when more
> requests are handled by the webserver). However, by ignoring the problem,
> the purpose of perform_idle_server_maintenance() would be lost!!
>
> Please respond with your thought, and if there are no objections, I will go
> ahead and post a patch for SOLUTION 1 sometime soon.
>
> Thanks,
> Mohan
>
> ************************* Start PATCH A ********************************
> --- server/mpm/threaded/threaded.c.orig Tue Jul 3 06:58:10 2001
> +++ server/mpm/threaded/threaded.c Fri Jul 13 18:28:44 2001
> @@ -495,7 +495,7 @@
> }
>
> /* Sets workers_may_exit if we received a character on the pipe_of_death */
> -static void check_pipe_of_death(void)
> +static void check_pipe_of_death(int process_slot)
> {
> apr_lock_acquire(pipe_of_death_mutex);
> if (!workers_may_exit) {
> @@ -511,6 +511,7 @@
> else {
> /* It won the lottery (or something else is very
> * wrong). Embrace death with open arms. */
> + ap_scoreboard_image->parent[process_slot].process_status =
> SB_IDLE_DIE;
> workers_may_exit = 1;
> }
> }
> @@ -584,7 +585,7 @@
> if (event & APR_POLLIN) {
> /* A process got a signal on the shutdown pipe. Check if
> we're
> * the lucky process to die. */
> - check_pipe_of_death();
> + check_pipe_of_death(process_slot);
> continue;
> }
>
> @@ -972,6 +973,9 @@
> int status = SERVER_DEAD;
> int any_dying_threads = 0;
> int any_dead_threads = 0;
>
> +
> + if (ap_scoreboard_image->parent[i].process_status == SB_IDLE_DIE)
> + continue;
>
> if (i >= ap_max_daemons_limit && free_length == idle_spawn_rate)
> break;
> ************************* End PATCH A ********************************
_____________________________________________________________________________
Ryan Bloom [EMAIL PROTECTED]
Covalent Technologies [EMAIL PROTECTED]
-----------------------------------------------------------------------------
