mpm_event,worker}: Really perform_idle_server_maintenance() per bucket.

mpm_{event,worker}: No need to pass num_buckets to perform_idle_server_maintenance() and server_main_loop().

mpm_worker: the last used scoreboard slot depends on pid != 0 only.

Align max_daemons_limit value on all MPMs.

That's the number of scoreboard slots in use, so it shouldn't be negative.
Fix off by one in OS/2 too.

Submitted by: ylavic <ylavic.dev@gmail.com>

Github: closes #274


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1894291 13f79535-47bb-0310-9956-ffa450edef68

4 files changed, 54 insertions, 50 deletions

diff --git a/server/mpm/event/event.c b/server/mpm/event/event.c
index ac7940d16d..cc3fa59fc9 100644
--- a/server/mpm/event/event.c
+++ b/server/mpm/event/event.c
@@ -3090,41 +3090,39 @@ static void startup_children(int number_to_start)
     }
 }
 
-static void perform_idle_server_maintenance(int child_bucket, int num_buckets)
+static void perform_idle_server_maintenance(int child_bucket)
 {
-    int i, j;
+    int num_buckets = retained->mpm->num_buckets;
     int idle_thread_count = 0;
-    worker_score *ws;
     process_score *ps;
     int free_length = 0;
     int free_slots[MAX_SPAWN_RATE];
     int last_non_dead = -1;
     int active_thread_count = 0;
+    int i, j;
 
     for (i = 0; i < server_limit; ++i) {
-        /* Initialization to satisfy the compiler. It doesn't know
-         * that threads_per_child is always > 0 */
-        int status = SERVER_DEAD;
-        int child_threads_active = 0;
-        int bucket = i % num_buckets;
-
+        if (num_buckets > 1 && (i % num_buckets) != child_bucket) {
+            /* We only care about child_bucket in this call */
+            continue;
+        }
         if (i >= retained->max_daemons_limit &&
             free_length == retained->idle_spawn_rate[child_bucket]) {
             /* short cut if all active processes have been examined and
              * enough empty scoreboard slots have been found
              */
-
             break;
         }
+
         ps = &ap_scoreboard_image->parent[i];
         if (ps->pid != 0) {
+            int child_threads_active = 0;
             if (ps->quiescing == 1) {
                 ps->quiescing = 2;
                 active_daemons--;
             }
             for (j = 0; j < threads_per_child; j++) {
-                ws = &ap_scoreboard_image->servers[i][j];
-                status = ws->status;
+                int status = ap_scoreboard_image->servers[i][j].status;
 
                 /* We consider a starting server as idle because we started it
                  * at least a cycle ago, and if it still hasn't finished starting
@@ -3133,26 +3131,26 @@ static void perform_idle_server_maintenance(int child_bucket, int num_buckets)
                  * This depends on the ordering of SERVER_READY and SERVER_STARTING.
                  */
                 if (status <= SERVER_READY && !ps->quiescing && !ps->not_accepting
-                    && ps->generation == retained->mpm->my_generation
-                    && bucket == child_bucket)
-                {
+                    && ps->generation == retained->mpm->my_generation) {
                     ++idle_thread_count;
                 }
                 if (status >= SERVER_READY && status < SERVER_GRACEFUL) {
                     ++child_threads_active;
                 }
             }
+            active_thread_count += child_threads_active;
+            if (child_threads_active == threads_per_child) {
+                had_healthy_child = 1;
+            }
             last_non_dead = i;
         }
-        active_thread_count += child_threads_active;
-        if (!ps->pid
-                && bucket == child_bucket
-                && free_length < retained->idle_spawn_rate[child_bucket])
+        else if (free_length < retained->idle_spawn_rate[child_bucket]) {
             free_slots[free_length++] = i;
-        else if (child_threads_active == threads_per_child)
-            had_healthy_child = 1;
+        }
     }
 
+    retained->max_daemons_limit = last_non_dead + 1;
+
     if (retained->sick_child_detected) {
         if (had_healthy_child) {
             /* Assume this is a transient error, even though it may not be.  Leave
@@ -3161,6 +3159,10 @@ static void perform_idle_server_maintenance(int child_bucket, int num_buckets)
              */
             retained->sick_child_detected = 0;
         }
+        else if (child_bucket < num_buckets - 1) {
+            /* check for had_healthy_child up to the last child bucket */
+            return;
+        }
         else {
             /* looks like a basket case, as no child ever fully initialized; give up.
              */
@@ -3176,8 +3178,6 @@ static void perform_idle_server_maintenance(int child_bucket, int num_buckets)
         }
     }
 
-    retained->max_daemons_limit = last_non_dead + 1;
-
     if (idle_thread_count > max_spare_threads / num_buckets)
     {
         /*
@@ -3217,7 +3217,7 @@ static void perform_idle_server_maintenance(int child_bucket, int num_buckets)
         }
     }
     else if (idle_thread_count < min_spare_threads / num_buckets) {
-        if (active_thread_count >= max_workers) {
+        if (active_thread_count >= max_workers / num_buckets) {
             if (0 == idle_thread_count) { 
                 if (!retained->maxclients_reported) {
                     ap_log_error(APLOG_MARK, APLOG_ERR, 0, ap_server_conf, APLOGNO(00484)
@@ -3302,8 +3302,9 @@ static void perform_idle_server_maintenance(int child_bucket, int num_buckets)
     }
 }
 
-static void server_main_loop(int remaining_children_to_start, int num_buckets)
+static void server_main_loop(int remaining_children_to_start)
 {
+    int num_buckets = retained->mpm->num_buckets;
     int child_slot;
     apr_exit_why_e exitwhy;
     int status, processed_status;
@@ -3412,7 +3413,7 @@ static void server_main_loop(int remaining_children_to_start, int num_buckets)
         }
 
         for (i = 0; i < num_buckets; i++) {
-            perform_idle_server_maintenance(i, num_buckets);
+            perform_idle_server_maintenance(i);
         }
     }
 }
@@ -3591,7 +3592,7 @@ static int event_run(apr_pool_t * _pconf, apr_pool_t * plog, server_rec * s)
 
     retained->mpm->mpm_state = AP_MPMQ_RUNNING;
 
-    server_main_loop(remaining_children_to_start, num_buckets);
+    server_main_loop(remaining_children_to_start);
     retained->mpm->mpm_state = AP_MPMQ_STOPPING;
 
     if (retained->mpm->shutdown_pending && retained->mpm->is_ungraceful) {
@@ -3796,7 +3797,6 @@ static int event_pre_config(apr_pool_t * pconf, apr_pool_t * plog,
         retained = ap_retained_data_create(userdata_key, sizeof(*retained));
         retained->mpm = ap_unixd_mpm_get_retained_data();
         retained->mpm->baton = retained;
-        retained->max_daemons_limit = -1;
         if (retained->mpm->module_loads) {
             test_atomics = 1;
         }
diff --git a/server/mpm/mpmt_os2/mpmt_os2.c b/server/mpm/mpmt_os2/mpmt_os2.c
index 77fc3e7f05..b3adb03adb 100644
--- a/server/mpm/mpmt_os2/mpmt_os2.c
+++ b/server/mpm/mpmt_os2/mpmt_os2.c
@@ -79,7 +79,7 @@ int ap_min_spare_threads = 0;
 int ap_max_spare_threads = 0;
 
 /* Keep track of a few interesting statistics */
-int ap_max_daemons_limit = -1;
+int ap_max_daemons_limit = 0;
 
 /* volatile just in case */
 static int volatile shutdown_pending;
@@ -344,8 +344,8 @@ static void spawn_child(int slot)
                      "error spawning child, slot %d", slot);
     }
 
-    if (ap_max_daemons_limit < slot) {
-        ap_max_daemons_limit = slot;
+    if (slot + 1 > ap_max_daemons_limit) {
+        ap_max_daemons_limit = slot + 1;
     }
 
     ap_scoreboard_image->parent[slot].pid = proc_rc.codeTerminate;
diff --git a/server/mpm/prefork/prefork.c b/server/mpm/prefork/prefork.c
index dfbc175585..ef5685fab9 100644
--- a/server/mpm/prefork/prefork.c
+++ b/server/mpm/prefork/prefork.c
@@ -1318,7 +1318,6 @@ static int prefork_pre_config(apr_pool_t *p, apr_pool_t *plog, apr_pool_t *ptemp
         retained = ap_retained_data_create(userdata_key, sizeof(*retained));
         retained->mpm = ap_unixd_mpm_get_retained_data();
         retained->mpm->baton = retained;
-        retained->max_daemons_limit = -1;
         retained->idle_spawn_rate = 1;
     }
     else if (retained->mpm->baton != retained) {
diff --git a/server/mpm/worker/worker.c b/server/mpm/worker/worker.c
index 12708c43b6..ddcde0f879 100644
--- a/server/mpm/worker/worker.c
+++ b/server/mpm/worker/worker.c
@@ -1373,11 +1373,10 @@ static void startup_children(int number_to_start)
     }
 }
 
-static void perform_idle_server_maintenance(int child_bucket, int num_buckets)
+static void perform_idle_server_maintenance(int child_bucket)
 {
-    int i, j;
+    int num_buckets = retained->mpm->num_buckets;
     int idle_thread_count;
-    worker_score *ws;
     process_score *ps;
     int free_length;
     int totally_free_length = 0;
@@ -1385,6 +1384,7 @@ static void perform_idle_server_maintenance(int child_bucket, int num_buckets)
     int last_non_dead;
     int total_non_dead;
     int active_thread_count = 0;
+    int i, j;
 
     /* initialize the free_list */
     free_length = 0;
@@ -1396,13 +1396,15 @@ static void perform_idle_server_maintenance(int child_bucket, int num_buckets)
     for (i = 0; i < ap_daemons_limit; ++i) {
         /* Initialization to satisfy the compiler. It doesn't know
          * that threads_per_child is always > 0 */
-        int status = SERVER_DEAD;
         int any_dying_threads = 0;
         int any_dead_threads = 0;
         int all_dead_threads = 1;
         int child_threads_active = 0;
-        int bucket = i % num_buckets;
 
+        if (num_buckets > 1 && (i % num_buckets) != child_bucket) {
+            /* We only care about child_bucket in this call */
+            continue;
+        }
         if (i >= retained->max_daemons_limit &&
             totally_free_length == retained->idle_spawn_rate[child_bucket]) {
             /* short cut if all active processes have been examined and
@@ -1412,8 +1414,7 @@ static void perform_idle_server_maintenance(int child_bucket, int num_buckets)
         }
         ps = &ap_scoreboard_image->parent[i];
         for (j = 0; j < threads_per_child; j++) {
-            ws = &ap_scoreboard_image->servers[i][j];
-            status = ws->status;
+            int status = ap_scoreboard_image->servers[i][j].status;
 
             /* XXX any_dying_threads is probably no longer needed    GLA */
             any_dying_threads = any_dying_threads ||
@@ -1433,8 +1434,7 @@ static void perform_idle_server_maintenance(int child_bucket, int num_buckets)
                                    loop if no pid?  not much else matters */
                 if (status <= SERVER_READY &&
                         !ps->quiescing &&
-                        ps->generation == retained->mpm->my_generation &&
-                        bucket == child_bucket) {
+                        ps->generation == retained->mpm->my_generation) {
                     ++idle_thread_count;
                 }
                 if (status >= SERVER_READY && status < SERVER_GRACEFUL) {
@@ -1444,7 +1444,6 @@ static void perform_idle_server_maintenance(int child_bucket, int num_buckets)
         }
         active_thread_count += child_threads_active;
         if (any_dead_threads
-                && bucket == child_bucket
                 && totally_free_length < retained->idle_spawn_rate[child_bucket]
                 && free_length < max_spawn_rate_per_bucket
                 && (!ps->pid               /* no process in the slot */
@@ -1472,11 +1471,15 @@ static void perform_idle_server_maintenance(int child_bucket, int num_buckets)
         }
         /* XXX if (!ps->quiescing)     is probably more reliable  GLA */
         if (!any_dying_threads) {
-            last_non_dead = i;
             ++total_non_dead;
         }
+        if (ps->pid != 0) {
+            last_non_dead = i;
+        }
     }
 
+    retained->max_daemons_limit = last_non_dead + 1;
+
     if (retained->sick_child_detected) {
         if (had_healthy_child) {
             /* Assume this is a transient error, even though it may not be.  Leave
@@ -1485,6 +1488,10 @@ static void perform_idle_server_maintenance(int child_bucket, int num_buckets)
              */
             retained->sick_child_detected = 0;
         }
+        else if (child_bucket < num_buckets - 1) {
+            /* check for had_healthy_child up to the last child bucket */
+            return;
+        }
         else {
             /* looks like a basket case, as no child ever fully initialized; give up.
              */
@@ -1500,8 +1507,6 @@ static void perform_idle_server_maintenance(int child_bucket, int num_buckets)
         }
     }
 
-    retained->max_daemons_limit = last_non_dead + 1;
-
     if (idle_thread_count > max_spare_threads / num_buckets) {
         /* Kill off one child */
         ap_mpm_podx_signal(retained->buckets[child_bucket].pod,
@@ -1512,7 +1517,7 @@ static void perform_idle_server_maintenance(int child_bucket, int num_buckets)
         /* terminate the free list */
         if (free_length == 0) { /* scoreboard is full, can't fork */
 
-            if (active_thread_count >= ap_daemons_limit * threads_per_child) {
+            if (active_thread_count >= max_workers / num_buckets) {
                 /* no threads are "inactive" - starting, stopping, etc. */
                 /* have we reached MaxRequestWorkers, or just getting close? */
                 if (0 == idle_thread_count) {
@@ -1579,8 +1584,9 @@ static void perform_idle_server_maintenance(int child_bucket, int num_buckets)
     }
 }
 
-static void server_main_loop(int remaining_children_to_start, int num_buckets)
+static void server_main_loop(int remaining_children_to_start)
 {
+    int num_buckets = retained->mpm->num_buckets;
     ap_generation_t old_gen;
     int child_slot;
     apr_exit_why_e exitwhy;
@@ -1694,7 +1700,7 @@ static void server_main_loop(int remaining_children_to_start, int num_buckets)
         }
 
         for (i = 0; i < num_buckets; i++) {
-            perform_idle_server_maintenance(i, num_buckets);
+            perform_idle_server_maintenance(i);
         }
     }
 }
@@ -1889,7 +1895,7 @@ static int worker_run(apr_pool_t *_pconf, apr_pool_t *plog, server_rec *s)
                 apr_proc_mutex_defname());
     retained->mpm->mpm_state = AP_MPMQ_RUNNING;
 
-    server_main_loop(remaining_children_to_start, num_buckets);
+    server_main_loop(remaining_children_to_start);
     retained->mpm->mpm_state = AP_MPMQ_STOPPING;
 
     if (retained->mpm->shutdown_pending && retained->mpm->is_ungraceful) {
@@ -2045,7 +2051,6 @@ static int worker_pre_config(apr_pool_t *pconf, apr_pool_t *plog,
         retained = ap_retained_data_create(userdata_key, sizeof(*retained));
         retained->mpm = ap_unixd_mpm_get_retained_data();
         retained->mpm->baton = retained;
-        retained->max_daemons_limit = -1;
     }
     else if (retained->mpm->baton != retained) {
         /* If the MPM changes on restart, be ungraceful */