You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by yl...@apache.org on 2020/12/11 14:49:13 UTC

svn commit: r1884318 - in /httpd/httpd/branches/2.4.x: ./ modules/http2/h2_workers.c

Author: ylavic
Date: Fri Dec 11 14:49:12 2020
New Revision: 1884318

URL: http://svn.apache.org/viewvc?rev=1884318&view=rev
Log:
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=<optimized out>) 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=<optimized out>) 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=<optimized out>) at memory/unix/apr_pools.c:1915
#6  0x00007f486502085c in apr_pool_destroy_debug (pool=<optimized out>, file_line=<optimized out>) 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=<optimized out>, 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=<optimized out>, argv=<optimized out>) 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=<optimized out>, mutex=<optimized out>) 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=<optimized out>, file_line=<optimized out>) 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

Modified:
    httpd/httpd/branches/2.4.x/   (props changed)
    httpd/httpd/branches/2.4.x/modules/http2/h2_workers.c

Propchange: httpd/httpd/branches/2.4.x/
------------------------------------------------------------------------------
  Merged /httpd/httpd/trunk:r1884169-1884170

Modified: httpd/httpd/branches/2.4.x/modules/http2/h2_workers.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/http2/h2_workers.c?rev=1884318&r1=1884317&r2=1884318&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/modules/http2/h2_workers.c (original)
+++ httpd/httpd/branches/2.4.x/modules/http2/h2_workers.c Fri Dec 11 14:49:12 2020
@@ -275,7 +275,7 @@ static apr_status_t workers_pool_cleanup
     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
     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
         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;