Hi Stefan, On Sun, Feb 5, 2017 at 10:39 PM, Stefan Fritsch <[email protected]> wrote: > > apr_allocator uses a mutex to be thread safe. Pools use this mutex also to > protect sub-pool creation, cleanup registering, etc. When apr creates the > initial allocator and global_pool in apr_pool_initialize(), it also creates a > mutex for this allocator. But mpm_event creates a new allocator for each > transaction pool and does not create a mutex for it. And gdb shows that in > listener_thread() at the get_worker() call, ptrans->allocator->mutex is NULL > (checked in 2.4.25).
Howesome catch! > > Shouldn't we allocate a mutex there and call apr_allocator_mutex_set()? If no, > why not. If yes, why does it still work as well as it does right now? Or could > the lacking mutex explain some weird segfaults? I think it always worked with HTTP/1 because we never race on request create/destroy (i.e. on creation/destruction of ptrans' subpools) nor, AFAICT, with subpools of the request (usually used by the same thread). We do have concurrent threads with subpools but they are created from pconf/pchild (hence mutexed). With HTTP/2 things get complicated because there are multiple subpools (of subpools) of ptrans (slave connections, mplx, ...) concurrently created/destroyed, and that obviously can't work without a mutex. Maybe since 2.4.25, and the changes wrt kept alive connections early closing on graceful (or/and the mpm event wakeup patch, not sure whether s.priebe@ finally runs mpm_event w/ or w/o it applied), we get more concurrency with ptrans being destroyed "out of band" (while h2 still has ongoing work). Anyway, it also seems to me that we should make ptrans mutexed, and so do we for all allocators dedicated to pools created in mod_http2. Regards, Yann.
