From: Yann Ylavic Date: Thu, 11 Jul 2024 13:48:18 +0000 (+0000) Subject: mpm_event: Simplify pollset "good methods" vs APR_POLLSET_WAKEABLE. X-Git-Tag: 2.4.62-rc1-candidate~17 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=90052684a982dfefcbdfb1dadc6b97014192f7ac;p=thirdparty%2Fapache%2Fhttpd.git mpm_event: Simplify pollset "good methods" vs APR_POLLSET_WAKEABLE. * server/mpm/event/event.c(setup_threads_runtime): Simplify pollset creation code. All pollset "good methods" implement APR_POLLSET_WAKEABLE and wake-ability is quite important for MPM event's correctness anyway so simplify code around pollset creation so as not to suggest that APR_POLLSET_NODEFAULT if favored against APR_POLLSET_WAKEABLE. While at it account for the wakeup pipe in the pollset_size since not all pollset methods seem to do it internally in APR. mpm_worker: Fix AH00045 about children processes not terminating timely. * server/mpm/worker/worker.c(setup_threads_runtime): Create pollset with APR_POLLSET_WAKEABLE to be able to wake up the listener when stopping. * server/mpm/worker/worker.c(wakeup_listener): Wake up the listener using the wakeup pipe (apr_pollset_wakeup). * server/mpm/worker/worker.c(join_workers): Like mpm_event, don't depend on `pthread_kill(listener_thread, 0)` to check whether the listener has exited (this does not work on some systems), but use the "dying" global variable instead which is set by the listener just before exiting. mpm_event,mpm_worker: Comment about pollset sizing when APR_POLLSET_WAKEABLE. Follow up to r1916925 and r1916926. Merges r1916925, r1916926, r1916929 from trunk Reviewed by: rjung, ylavic, jorton git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1919134 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/changes-entries/worker_exit.txt b/changes-entries/worker_exit.txt new file mode 100644 index 00000000000..5a2e712e95d --- /dev/null +++ b/changes-entries/worker_exit.txt @@ -0,0 +1,2 @@ + *) mpm_worker: Fix possible warning (AH00045) about children processes not + terminating timely. [Yann Ylavic] diff --git a/server/mpm/event/event.c b/server/mpm/event/event.c index 3672f449634..7e7a7e91baf 100644 --- a/server/mpm/event/event.c +++ b/server/mpm/event/event.c @@ -2319,31 +2319,32 @@ static void setup_threads_runtime(void) clean_child_exit(APEXIT_CHILDFATAL); } - /* Create the main pollset */ + /* Create the main pollset. When APR_POLLSET_WAKEABLE is asked we account + * for the wakeup pipe explicitely with pollset_size+1 because some pollset + * implementations don't do it implicitely in APR. + */ pollset_flags = APR_POLLSET_THREADSAFE | APR_POLLSET_NOCOPY | - APR_POLLSET_NODEFAULT | APR_POLLSET_WAKEABLE; + APR_POLLSET_WAKEABLE | APR_POLLSET_NODEFAULT; for (i = 0; i < sizeof(good_methods) / sizeof(good_methods[0]); i++) { - rv = apr_pollset_create_ex(&event_pollset, pollset_size, pruntime, + rv = apr_pollset_create_ex(&event_pollset, pollset_size + 1, pruntime, pollset_flags, good_methods[i]); if (rv == APR_SUCCESS) { listener_is_wakeable = 1; break; } } - if (rv != APR_SUCCESS) { - pollset_flags &= ~APR_POLLSET_WAKEABLE; - for (i = 0; i < sizeof(good_methods) / sizeof(good_methods[0]); i++) { - rv = apr_pollset_create_ex(&event_pollset, pollset_size, pruntime, - pollset_flags, good_methods[i]); - if (rv == APR_SUCCESS) { - break; - } - } - } if (rv != APR_SUCCESS) { pollset_flags &= ~APR_POLLSET_NODEFAULT; - rv = apr_pollset_create(&event_pollset, pollset_size, pruntime, + rv = apr_pollset_create(&event_pollset, pollset_size + 1, pruntime, pollset_flags); + if (rv == APR_SUCCESS) { + listener_is_wakeable = 1; + } + else { + pollset_flags &= ~APR_POLLSET_WAKEABLE; + rv = apr_pollset_create(&event_pollset, pollset_size, pruntime, + pollset_flags); + } } if (rv != APR_SUCCESS) { ap_log_error(APLOG_MARK, APLOG_ERR, rv, ap_server_conf, APLOGNO(03103) diff --git a/server/mpm/worker/worker.c b/server/mpm/worker/worker.c index 7b572bd14c9..315371de121 100644 --- a/server/mpm/worker/worker.c +++ b/server/mpm/worker/worker.c @@ -125,10 +125,11 @@ static int max_workers = 0; static int server_limit = 0; static int thread_limit = 0; static int had_healthy_child = 0; -static int dying = 0; +static volatile int dying = 0; static int workers_may_exit = 0; static int start_thread_may_exit = 0; static int listener_may_exit = 0; +static int listener_is_wakeable = 0; /* Pollset supports APR_POLLSET_WAKEABLE */ static int requests_this_child; static int num_listensocks = 0; static int resource_shortage = 0; @@ -272,6 +273,15 @@ static void close_worker_sockets(void) static void wakeup_listener(void) { listener_may_exit = 1; + + /* Unblock the listener if it's poll()ing */ + if (worker_pollset && listener_is_wakeable) { + apr_pollset_wakeup(worker_pollset); + } + + /* unblock the listener if it's waiting for a worker */ + ap_queue_info_term(worker_queue_info); + if (!listener_os_thread) { /* XXX there is an obscure path that this doesn't handle perfectly: * right after listener thread is created but before @@ -280,10 +290,6 @@ static void wakeup_listener(void) */ return; } - - /* unblock the listener if it's waiting for a worker */ - ap_queue_info_term(worker_queue_info); - /* * we should just be able to "kill(ap_my_pid, LISTENER_SIGNAL)" on all * platforms and wake up the listener thread since it is the only thread @@ -861,6 +867,7 @@ static void create_listener_thread(thread_starter *ts) static void setup_threads_runtime(void) { ap_listen_rec *lr; + int pollset_flags; apr_status_t rv; /* All threads (listener, workers) and synchronization objects (queues, @@ -893,9 +900,21 @@ static void setup_threads_runtime(void) clean_child_exit(APEXIT_CHILDFATAL); } - /* Create the main pollset */ - rv = apr_pollset_create(&worker_pollset, num_listensocks, pruntime, - APR_POLLSET_NOCOPY); + /* Create the main pollset. When APR_POLLSET_WAKEABLE is asked we account + * for the wakeup pipe explicitely with num_listensocks+1 because some + * pollset implementations don't do it implicitely in APR. + */ + pollset_flags = APR_POLLSET_NOCOPY | APR_POLLSET_WAKEABLE; + rv = apr_pollset_create(&worker_pollset, num_listensocks + 1, pruntime, + pollset_flags); + if (rv == APR_SUCCESS) { + listener_is_wakeable = 1; + } + else { + pollset_flags &= ~APR_POLLSET_WAKEABLE; + rv = apr_pollset_create(&worker_pollset, num_listensocks, pruntime, + pollset_flags); + } if (rv != APR_SUCCESS) { ap_log_error(APLOG_MARK, APLOG_EMERG, rv, ap_server_conf, APLOGNO(03285) "Couldn't create pollset in thread;" @@ -1031,19 +1050,17 @@ static void join_workers(apr_thread_t *listener, apr_thread_t **threads, */ iter = 0; - while (iter < 10 && -#ifdef HAVE_PTHREAD_KILL - pthread_kill(*listener_os_thread, 0) -#else - kill(ap_my_pid, 0) -#endif - == 0) { - /* listener not dead yet */ - apr_sleep(apr_time_make(0, 500000)); + while (!dying) { + apr_sleep(apr_time_from_msec(500)); + if (dying || ++iter > 10) { + break; + } + /* listener has not stopped accepting yet */ + ap_log_error(APLOG_MARK, APLOG_TRACE1, 0, ap_server_conf, + "listener has not stopped accepting yet (%d iter)", iter); wakeup_listener(); - ++iter; } - if (iter >= 10) { + if (iter > 10) { ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, ap_server_conf, APLOGNO(00276) "the listener thread didn't exit"); }