From: Yann Ylavic Date: Fri, 11 Dec 2020 14:49:12 +0000 (+0000) Subject: Merge r1884169, r1884170 from trunk: X-Git-Tag: 2.4.47~206 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=fd70b17ea50e9f3e9243d0b5818a2cc63dee2eb1;p=thirdparty%2Fapache%2Fhttpd.git Merge r1884169, r1884170 from trunk: mod_http2: Rename server_pool as pchild in h2_workers_create() To clarify which parent pool the workers threads have. And add a comment about workers_pool_cleanup()'s role and when it runs. No functional change. mod_http2: stop/wait the workers threads before their pool is killed. There shouldn't be any worker thread active when pchild is destroyed (thus each thread's pool), so register workers_pool_cleanup as a pre_cleanup of pchild. This is to avoid races like the below stacktrace, where slot_run() threads are still running when clean_child_exit() is called. Thread 23 (Thread 0x7f4865b79800 (LWP 3740)): #0 0x00007f4864dec449 in pthread_cond_destroy@@GLIBC_2.3.2 () from /lib/x86_64-linux-gnu/libpthread.so.0 #1 0x00007f4865020117 in run_cleanups (cref=) at memory/unix/apr_pools.c:2629 #2 pool_clear_debug (pool=pool@entry=0x558a5297e4a0, file_line=0x558a5237456b "event.c:757") at memory/unix/apr_pools.c:1830 #3 0x00007f486501ffee in pool_destroy_debug (pool=0x558a5297e4a0, file_line=) at memory/unix/apr_pools.c:1915 #4 0x00007f48650200f0 in pool_clear_debug (pool=pool@entry=0x558a52a41070, file_line=0x558a5237456b "event.c:757") at memory/unix/apr_pools.c:1827 #5 0x00007f486501ffee in pool_destroy_debug (pool=0x558a52a41070, file_line=) at memory/unix/apr_pools.c:1915 #6 0x00007f486502085c in apr_pool_destroy_debug (pool=, file_line=) at memory/unix/apr_pools.c:1957 #7 0x0000558a52326cfc in clean_child_exit (code=0) at event.c:757 #8 0x0000558a52327969 in child_main (child_num_arg=child_num_arg@entry=1, child_bucket=child_bucket@entry=0) at event.c:2926 #9 0x0000558a52327ce5 in make_child (s=0x558a52c9f840, slot=slot@entry=1, bucket=0) at event.c:2992 #10 0x0000558a52327d4c in startup_children (number_to_start=2, number_to_start@entry=3) at event.c:3015 #11 0x0000558a523289ac in event_run (_pconf=, plog=0x558a5273ce00, s=0x558a52c9f840) at event.c:3374 #12 0x0000558a5233e91e in ap_run_mpm (pconf=0x558a5270cbe0, plog=0x558a5273ce00, s=0x558a52c9f840) at mpm_common.c:100 #13 0x0000558a5231b763 in main (argc=, argv=) at main.c:844 Thread 2 (Thread 0x7f4840b70700 (LWP 3836)): #0 0x00007f4864dec9f3 in pthread_cond_wait@@GLIBC_2.3.2 () from /lib/x86_64-linux-gnu/libpthread.so.0 #1 0x00007f486501f65d in apr_thread_cond_wait (cond=, mutex=) at locks/unix/thread_cond.c:68 #2 0x00007f484e14ae4a in get_next (slot=0x558a528d5fe0) at h2_workers.c:209 #3 slot_run (thread=0x558a52828b30, wctx=0x558a528d5fe0) at h2_workers.c:228 #4 0x00007f4864de66db in start_thread () from /lib/x86_64-linux-gnu/libpthread.so.0 #5 0x00007f4864b0f88f in clone () from /lib/x86_64-linux-gnu/libc.so.6 Thread 1 (Thread 0x7f4841b72700 (LWP 3834)): #0 0x00007f4864a2ce97 in raise () from /lib/x86_64-linux-gnu/libc.so.6 #1 0x00007f4864a2e801 in abort () from /lib/x86_64-linux-gnu/libc.so.6 #2 0x00007f4865020865 in apr_pool_destroy_debug (pool=, file_line=) at memory/unix/apr_pools.c:1955 #3 0x00007f486502b536 in apr_thread_exit (thd=thd@entry=0x558a52ba8980, retval=retval@entry=0) at threadproc/unix/thread.c:206 #4 0x00007f484e14aec6 in slot_run (thread=0x558a52ba8980, wctx=0x558a528d6060) at h2_workers.c:248 #5 0x00007f4864de66db in start_thread () from /lib/x86_64-linux-gnu/libpthread.so.0 #6 0x00007f4864b0f88f in clone () from /lib/x86_64-linux-gnu/libc.so.6 Submitted by: ylavic Reviewed by: ylavic, jorton, covener git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1884318 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/modules/http2/h2_workers.c b/modules/http2/h2_workers.c index 96c20a894f5..6ead3d735f8 100644 --- a/modules/http2/h2_workers.c +++ b/modules/http2/h2_workers.c @@ -275,7 +275,7 @@ static apr_status_t workers_pool_cleanup(void *data) return APR_SUCCESS; } -h2_workers *h2_workers_create(server_rec *s, apr_pool_t *server_pool, +h2_workers *h2_workers_create(server_rec *s, apr_pool_t *pchild, int min_workers, int max_workers, int idle_secs) { @@ -285,14 +285,14 @@ h2_workers *h2_workers_create(server_rec *s, apr_pool_t *server_pool, int i, n; ap_assert(s); - ap_assert(server_pool); + ap_assert(pchild); /* let's have our own pool that will be parent to all h2_worker * instances we create. This happens in various threads, but always * guarded by our lock. Without this pool, all subpool creations would * happen on the pool handed to us, which we do not guard. */ - apr_pool_create(&pool, server_pool); + apr_pool_create(&pool, pchild); apr_pool_tag(pool, "h2_workers"); workers = apr_pcalloc(pool, sizeof(h2_workers)); if (!workers) { @@ -363,7 +363,12 @@ h2_workers *h2_workers_create(server_rec *s, apr_pool_t *server_pool, workers->dynamic = (workers->worker_count < workers->max_workers); } if (status == APR_SUCCESS) { - apr_pool_pre_cleanup_register(pool, workers, workers_pool_cleanup); + /* Stop/join the workers threads when the MPM child exits (pchild is + * destroyed), and as a pre_cleanup of pchild thus before the threads + * pools (children of workers->pool) so that they are not destroyed + * before/under us. + */ + apr_pool_pre_cleanup_register(pchild, workers, workers_pool_cleanup); return workers; } return NULL;