From b0bf11b8e69ce5e8cfe9954853889b9c62b30e70 Mon Sep 17 00:00:00 2001 From: Stefan Fritsch Date: Sat, 20 Aug 2016 20:48:13 +0000 Subject: [PATCH] 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 --- docs/log-message-tags/next-number | 2 +- server/mpm/event/event.c | 80 ++++++------------------------- 2 files changed, 15 insertions(+), 67 deletions(-) diff --git a/docs/log-message-tags/next-number b/docs/log-message-tags/next-number index bbd257fd6f..169efa8bd2 100644 --- 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 96a02a6738..fbfaeec61e 100644 --- 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. -- 2.40.0