From a1a1d20b7629959346f54f7bc56cfc2721c89cf6 Mon Sep 17 00:00:00 2001 From: Ryan Bloom Date: Thu, 28 Jun 2001 05:00:59 +0000 Subject: [PATCH] Fix the big issue with the threaded MPM. We no longer stop dead if all processes are busy serving one long-lived request. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@89466 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES | 12 +++ STATUS | 17 +--- server/mpm/threaded/threaded.c | 159 ++++++++++++++++++++------------- 3 files changed, 111 insertions(+), 77 deletions(-) diff --git a/CHANGES b/CHANGES index 891e32d8b2..cf809421bb 100644 --- a/CHANGES +++ b/CHANGES @@ -1,5 +1,17 @@ Changes with Apache 2.0.19-dev + *) Fix problem with threaded MPM. The problem was that if each child + process was busy serving a single long-lived request and the server + was sent a graceful restart signal, the server would stop serving + requests. This would happen because each child process would wait to + die until the last thread was done, and the parent wouldn't spawn any + new children until a process died. Now, the parent looks at the fact + that the children are dying gracefully, and starts new children. + Those new children only start enough threads to compliment the number + of threads in the other child process that shares the same spot in + the scoreboard. In this way, we make sure to never go over + MaxClients. [Ryan Bloom] + *) modified mod_negotiation and mod_autoindex to speed up by almost a factor of two on apr_dir_read()-enhanced platforms, such as Win32 and OS2, by calling ap_sub_request_lookup_dirent() with the results diff --git a/STATUS b/STATUS index 80d66bb35a..878a214353 100644 --- a/STATUS +++ b/STATUS @@ -1,5 +1,5 @@ APACHE 2.0 STATUS: -*-text-*- -Last modified at [$Date: 2001/06/28 03:57:30 $] +Last modified at [$Date: 2001/06/28 05:00:51 $] Release: @@ -192,21 +192,6 @@ RELEASE SHOWSTOPPERS: to make it agree with the operation of the StartServers directive. - * A design problem with the scoreboard can cause the threaded - MPM to get in a state where it will no longer serve requests. - When MaxRequestsPerChild is hit, a threaded process will begin - allowing it's idle worker threads to exit. The child process - may have one (or a few) threads serving really long responses - over slow client connections, which will prevent the child - process from exiting. The problem is that the - perform_idle_server_maintenance code will NOT start a new - process to replace the dying child process until the dying child - has exited because the new child needs to use the old childs - space in the scoreboard. The scoreboard and - perform_idle_server_maintenance need to be redesigned. - Status: Several proposals discussed on new-httpd (April 16, 2001) - - RELEASE NON-SHOWSTOPPERS BUT WOULD BE REAL NICE TO WRAP THESE UP: * Port of mod_ssl to Apache 2.0: diff --git a/server/mpm/threaded/threaded.c b/server/mpm/threaded/threaded.c index ec79fda54f..d5f2b52b0c 100644 --- a/server/mpm/threaded/threaded.c +++ b/server/mpm/threaded/threaded.c @@ -111,6 +111,7 @@ static int ap_daemons_to_start=0; static int min_spare_threads=0; static int max_spare_threads=0; static int ap_daemons_limit=0; +static int dying = 0; static int workers_may_exit = 0; static int requests_this_child; static int num_listensocks = 0; @@ -124,6 +125,15 @@ typedef struct { apr_pool_t *tpool; /* "pthread" would be confusing */ } proc_info; +/* Structure used to pass information to the thread responsible for + * creating the rest of the threads. + */ +typedef struct { + apr_thread_t **threads; + int child_num_arg; + apr_threadattr_t *threadattr; +} thread_starter; + /* * The max child slot ever assigned, preserved across restarts. Necessary * to deal with MaxClients changes across SIGWINCH restarts. We use this @@ -633,8 +643,9 @@ static void * worker_thread(void * dummy) } apr_pool_destroy(tpool); - ap_update_child_status(process_slot, thread_slot, SERVER_DEAD, + ap_update_child_status(process_slot, thread_slot, (dying) ? SERVER_DEAD : SERVER_GRACEFUL, (request_rec *) NULL); + dying = 1; apr_lock_acquire(worker_thread_count_mutex); worker_thread_count--; if (worker_thread_count == 0) { @@ -657,16 +668,80 @@ static int check_signal(int signum) return 0; } -static void child_main(int child_num_arg) +static void *start_threads(void * dummy) { - apr_thread_t **threads; - apr_threadattr_t *thread_attr; + thread_starter *ts = dummy; + apr_thread_t **threads = ts->threads; + apr_threadattr_t *thread_attr = ts->threadattr; + int child_num_arg = ts->child_num_arg; int i; int my_child_num = child_num_arg; proc_info *my_info = NULL; ap_listen_rec *lr; apr_status_t rv; + int threads_created = 0; + + while (1) { + for (i=0; i < ap_threads_per_child; i++) { + int status = ap_scoreboard_image->servers[child_num_arg][i].status; + + if (status != SERVER_GRACEFUL && status != SERVER_DEAD) { + continue; + } + + my_info = (proc_info *)malloc(sizeof(proc_info)); + if (my_info == NULL) { + ap_log_error(APLOG_MARK, APLOG_ALERT, errno, ap_server_conf, + "malloc: out of memory"); + clean_child_exit(APEXIT_CHILDFATAL); + } + my_info->pid = my_child_num; + my_info->tid = i; + my_info->sd = 0; + apr_pool_create(&my_info->tpool, pchild); + + /* We are creating threads right now */ + (void) ap_update_child_status(my_child_num, i, SERVER_STARTING, + (request_rec *) NULL); + /* We let each thread update it's own scoreboard entry. This is + * done because it let's us deal with tid better. + */ + if ((rv = apr_thread_create(&threads[i], thread_attr, worker_thread, my_info, pchild))) { + ap_log_error(APLOG_MARK, APLOG_ALERT, rv, ap_server_conf, + "apr_thread_create: unable to create worker thread"); + /* In case system resources are maxxed out, we don't want + Apache running away with the CPU trying to fork over and + over and over again if we exit. */ + sleep(10); + clean_child_exit(APEXIT_CHILDFATAL); + } + threads_created++; + } + if (workers_may_exit || threads_created == ap_threads_per_child) { + break; + } + sleep(1); + } + + /* What state should this child_main process be listed as in the scoreboard...? + * ap_update_child_status(my_child_num, i, SERVER_STARTING, (request_rec *) NULL); + * + * This state should be listed separately in the scoreboard, in some kind + * of process_status, not mixed in with the worker threads' status. + * "life_status" is almost right, but it's in the worker's structure, and + * the name could be clearer. gla + */ +} +static void child_main(int child_num_arg) +{ + apr_thread_t **threads; + int i; + proc_info *my_info = NULL; + ap_listen_rec *lr; + apr_status_t rv; + thread_starter *ts; + apr_threadattr_t *thread_attr; ap_my_pid = getpid(); apr_pool_create(&pchild, pconf); @@ -727,47 +802,25 @@ static void child_main(int child_num_arg) NULL, pchild); apr_lock_create(&pipe_of_death_mutex, APR_MUTEX, APR_INTRAPROCESS, NULL, pchild); + + ts = apr_palloc(pchild, sizeof(*ts)); + apr_threadattr_create(&thread_attr, pchild); apr_threadattr_detach_set(thread_attr, 0); /* 0 means PTHREAD_CREATE_JOINABLE */ - for (i=0; i < ap_threads_per_child; i++) { - - my_info = (proc_info *)malloc(sizeof(proc_info)); - if (my_info == NULL) { - ap_log_error(APLOG_MARK, APLOG_ALERT, errno, ap_server_conf, - "malloc: out of memory"); - clean_child_exit(APEXIT_CHILDFATAL); - } - my_info->pid = my_child_num; - my_info->tid = i; - my_info->sd = 0; - apr_pool_create(&my_info->tpool, pchild); - - /* We are creating threads right now */ - (void) ap_update_child_status(my_child_num, i, SERVER_STARTING, - (request_rec *) NULL); - if ((rv = apr_thread_create(&threads[i], thread_attr, worker_thread, my_info, pchild))) { - ap_log_error(APLOG_MARK, APLOG_ALERT, rv, ap_server_conf, - "apr_thread_create: unable to create worker thread"); - /* In case system resources are maxxed out, we don't want - Apache running away with the CPU trying to fork over and - over and over again if we exit. */ - sleep(10); - clean_child_exit(APEXIT_CHILDFATAL); - } - /* We let each thread update it's own scoreboard entry. This is done - * because it let's us deal with tid better. - */ + ts->threads = threads; + ts->child_num_arg = child_num_arg; + ts->threadattr = thread_attr; + + if ((rv = apr_thread_create(&threads[i], thread_attr, start_threads, ts, pchild))) { + ap_log_error(APLOG_MARK, APLOG_ALERT, rv, ap_server_conf, + "apr_thread_create: unable to create worker thread"); + /* In case system resources are maxxed out, we don't want + Apache running away with the CPU trying to fork over and + over and over again if we exit. */ + sleep(10); + clean_child_exit(APEXIT_CHILDFATAL); } - - /* What state should this child_main process be listed as in the scoreboard...? - * ap_update_child_status(my_child_num, i, SERVER_STARTING, (request_rec *) NULL); - * - * This state should be listed separately in the scoreboard, in some kind - * of process_status, not mixed in with the worker threads' status. - * "life_status" is almost right, but it's in the worker's structure, and - * the name could be clearer. gla - */ apr_signal_thread(check_signal); @@ -784,7 +837,7 @@ static void child_main(int child_num_arg) apr_thread_join(&rv, threads[i]); } - free (threads); + free(threads); clean_child_exit(0); } @@ -803,10 +856,6 @@ static int make_child(server_rec *s, int slot) child_main(slot); } - /* Tag this slot as occupied so that perform_idle_server_maintenance - * doesn't try to steal it */ - (void) ap_update_child_status(slot, 0, SERVER_STARTING, (request_rec *) NULL); - if ((pid = fork()) == -1) { ap_log_error(APLOG_MARK, APLOG_ERR, errno, s, "fork: Unable to fork new process"); @@ -923,7 +972,6 @@ static void perform_idle_server_maintenance(void) * that ap_threads_per_child is always > 0 */ int status = SERVER_DEAD; int any_dying_threads = 0; - int all_dead_threads = 1; int idle_thread_addition = 0; if (i >= ap_max_daemons_limit && free_length == idle_spawn_rate) @@ -932,9 +980,7 @@ static void perform_idle_server_maintenance(void) ws = &ap_scoreboard_image->servers[i][j]; status = ws->status; - any_dying_threads = any_dying_threads || (status == SERVER_DEAD) - || (status == SERVER_GRACEFUL); - all_dead_threads = all_dead_threads && (status == SERVER_DEAD); + any_dying_threads = any_dying_threads || (status == SERVER_GRACEFUL); /* We consider a starting server as idle because we started it * at least a cycle ago, and if it still hasn't finished starting @@ -946,14 +992,12 @@ static void perform_idle_server_maintenance(void) ++idle_thread_addition; } } - if (all_dead_threads && free_length < idle_spawn_rate) { + if (any_dying_threads && free_length < idle_spawn_rate) { free_slots[free_length] = i; ++free_length; } - if (!all_dead_threads) { + if (!any_dying_threads) { last_non_dead = i; - } - if (!any_dying_threads) { ++total_non_dead; idle_thread_count += idle_thread_addition; } @@ -1218,13 +1262,6 @@ int ap_mpm_run(apr_pool_t *_pconf, apr_pool_t *plog, server_rec *s) * gracefully dealing with existing request. */ - for (i = 0; i < ap_daemons_limit; ++i) { - for (j = 0; j < ap_threads_per_child; j++) { - if (ap_scoreboard_image->servers[i][j].status != SERVER_DEAD) { - ap_scoreboard_image->servers[i][j].status = SERVER_GRACEFUL; - } - } - } } else { /* Kill 'em all. Since the child acts the same on the parents SIGTERM -- 2.40.0