mpm_event: don't re-use scoreboard slots that are still in use

This causes inconsistent data in the scoreboard (due to async
connections) and makes it difficult to determine what is going on.
Therefore it is not a useful fix for the scoreboard-full issues (PR
53555).

The consent on the dev list is that we should allocate/use more
scoreboard entries instead.

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

diff --git a/docs/log-message-tags/next-number b/docs/log-message-tags/next-number
index bbd257fd6f0dc5ced0f2f680291a549049fc260c..169efa8bd26e4837bafefa6e14c2abee4541f6bc 100644 (file)
--- a/docs/log-message-tags/next-number
+++ b/docs/log-message-tags/next-number
@@ -1 +1 @@
-3455
+3456

diff --git a/server/mpm/event/event.c b/server/mpm/event/event.c
index 96a02a6738e3bdd347e464e87e4fac38964b762d..fbfaeec61e9a93fc568ff81acac46bf7d7d543fd 100644 (file)
--- a/server/mpm/event/event.c
+++ b/server/mpm/event/event.c
@@ -625,27 +625,6 @@ static void event_note_child_started(int slot, pid_t pid)
                         retained->my_generation, slot, MPM_CHILD_STARTED);
 }
 
-static void event_note_child_lost_slot(int slot, pid_t newpid)
-{
-    ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, ap_server_conf, APLOGNO(00458)
-                 "pid %" APR_PID_T_FMT " taking over scoreboard slot from "
-                 "%" APR_PID_T_FMT "%s",
-                 newpid,
-                 ap_scoreboard_image->parent[slot].pid,
-                 ap_scoreboard_image->parent[slot].quiescing ?
-                 " (quiescing)" : "");
-    ap_run_child_status(ap_server_conf,
-                        ap_scoreboard_image->parent[slot].pid,
-                        ap_scoreboard_image->parent[slot].generation,
-                        slot, MPM_CHILD_LOST_SLOT);
-    /* Don't forget about this exiting child process, or we
-     * won't be able to kill it if it doesn't exit by the
-     * time the server is shut down.
-     */
-    ap_register_extra_mpm_process(ap_scoreboard_image->parent[slot].pid,
-                                  ap_scoreboard_image->parent[slot].generation);
-}
-
 static const char *event_get_name(void)
 {
     return "event";
@@ -2727,6 +2706,15 @@ static int make_child(server_rec * s, int slot, int bucket)
         retained->max_daemons_limit = slot + 1;
     }
 
+    if (ap_scoreboard_image->parent[slot].pid != 0) {
+        /* XXX replace with assert or remove ? */
+        ap_log_error(APLOG_MARK, APLOG_ERR, 0, ap_server_conf, APLOGNO(03455)
+                 "BUG: Scoreboard slot %d should be empty but is "
+                 "in use by pid %" APR_PID_T_FMT,
+                 slot, ap_scoreboard_image->parent[slot].pid);
+        return -1;
+    }
+
     if (one_process) {
         my_bucket = &all_buckets[0];
 
@@ -2780,13 +2768,6 @@ static int make_child(server_rec * s, int slot, int bucket)
         return -1;
     }
 
-    if (ap_scoreboard_image->parent[slot].pid != 0) {
-        /* This new child process is squatting on the scoreboard
-         * entry owned by an exiting child process, which cannot
-         * exit until all active requests complete.
-         */
-        event_note_child_lost_slot(slot, pid);
-    }
     ap_scoreboard_image->parent[slot].quiescing = 0;
     ap_scoreboard_image->parent[slot].not_accepting = 0;
     ap_scoreboard_image->parent[slot].bucket = bucket;
@@ -2817,7 +2798,6 @@ static void perform_idle_server_maintenance(int child_bucket, int num_buckets)
     worker_score *ws;
     process_score *ps;
     int free_length = 0;
-    int totally_free_length = 0;
     int free_slots[MAX_SPAWN_RATE];
     int last_non_dead = -1;
     int total_non_dead = 0;
@@ -2833,7 +2813,7 @@ static void perform_idle_server_maintenance(int child_bucket, int num_buckets)
         int child_threads_active = 0;
 
         if (i >= retained->max_daemons_limit &&
-            totally_free_length == retained->idle_spawn_rate[child_bucket]) {
+            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
              */
@@ -2872,28 +2852,9 @@ static void perform_idle_server_maintenance(int child_bucket, int num_buckets)
             }
         }
         active_thread_count += child_threads_active;
-        if (any_dead_threads
-            && totally_free_length < retained->idle_spawn_rate[child_bucket]
-            && free_length < MAX_SPAWN_RATE / num_buckets
-            && (!ps->pid      /* no process in the slot */
-                  || ps->quiescing)) {  /* or at least one is going away */
-            if (all_dead_threads) {
-                /* great! we prefer these, because the new process can
-                 * start more threads sooner.  So prioritize this slot
-                 * by putting it ahead of any slots with active threads.
-                 *
-                 * first, make room by moving a slot that's potentially still
-                 * in use to the end of the array
-                 */
-                free_slots[free_length] = free_slots[totally_free_length];
-                free_slots[totally_free_length++] = i;
-            }
-            else {
-                /* slot is still in use - back of the bus
-                 */
-                free_slots[free_length] = i;
-            }
-            ++free_length;
+        if (!ps->pid && 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;
@@ -2991,7 +2952,6 @@ 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)
 {
-    ap_generation_t old_gen;
     int child_slot;
     apr_exit_why_e exitwhy;
     int status, processed_status;
@@ -3055,24 +3015,12 @@ static void server_main_loop(int remaining_children_to_start, int num_buckets)
                     --remaining_children_to_start;
                 }
             }
-            else if (ap_unregister_extra_mpm_process(pid.pid, &old_gen) == 1) {
-
-                event_note_child_killed(-1, /* already out of the scoreboard */
-                                        pid.pid, old_gen);
-                if (processed_status == APEXIT_CHILDSICK
-                    && old_gen == retained->my_generation) {
-                    /* resource shortage, minimize the fork rate */
-                    for (i = 0; i < num_buckets; i++) {
-                        retained->idle_spawn_rate[i] = 1;
-                    }
-                }
 #if APR_HAS_OTHER_CHILD
-            }
             else if (apr_proc_other_child_alert(&pid, APR_OC_REASON_DEATH,
                                                 status) == 0) {
                 /* handled */
-#endif
             }
+#endif
             else if (retained->is_graceful) {
                 /* Great, we've probably just lost a slot in the
                  * scoreboard.  Somehow we don't know about this child.