From 4b0fef3c53fc788657cdee603c6662ae016c12f4 Mon Sep 17 00:00:00 2001 From: "Michiel W. Beijen" Date: Sun, 1 Nov 2026 16:04:09 +0100 Subject: [PATCH 1/3] mod_http3: Keep the child's threads off the child pool The threads of mod_http3 allocated from the child pool, which is not thread-safe, and kept running while it was destroyed: - The port acquirer thread, in every child that does not own the UDP port, created a socket in the child pool every 200 ms, racing with the main thread and never freeing it. - Under prefork the child pool's allocator has no mutex and is shared with the request pools, which the event thread raced with when it created connection pools. - prefork does not run the child_stopping hook when a child exits from a signal handler, so neither thread was stopped before the child pool was destroyed and exit() ran OpenSSL's cleanup under them. These crashed children with SIGSEGV on stop, mostly under prefork. Give mod_http3 a pool of its own with a locked allocator, give each port acquisition attempt a scratch pool that is cleared on retry, and stop the threads from a pre-cleanup of the child pool as well. Generated-by: Claude Code (Claude Opus 5.5) --- modules/http3/h3_server.c | 81 ++++++++++++++++++++++++++++++++++----- 1 file changed, 71 insertions(+), 10 deletions(-) diff --git a/modules/http3/h3_server.c b/modules/http3/h3_server.c index 9a7ef5fb121..4c74ef66b49 100644 --- a/modules/http3/h3_server.c +++ b/modules/http3/h3_server.c @@ -38,7 +38,7 @@ static apr_thread_t* port_acquire_thread = NULL; struct port_acquire_args { - apr_pool_t* pchild; + apr_pool_t* pool; server_rec* vhost; h3_server_conf* conf; }; @@ -46,12 +46,20 @@ struct port_acquire_args static void* APR_THREAD_FUNC port_acquire_thread_fn(apr_thread_t* thread H3_UNUSED, void* data) { struct port_acquire_args* args = data; + /* Each attempt allocates; clear them, or retrying leaks for the life + * of the child. A successful attempt's socket stays in ptry. */ + apr_pool_t* ptry = NULL; + if (apr_pool_create(&ptry, args->pool) != APR_SUCCESS) + { + return NULL; + } while (!child_stopping) { int udp_fd = -1; - apr_status_t rv = h3_socket_open(args->conf->h3_port, args->conf->h3_socket_buffer_size, args->pchild, &udp_fd); + apr_status_t rv = h3_socket_open(args->conf->h3_port, args->conf->h3_socket_buffer_size, ptry, &udp_fd); if (rv == APR_EAGAIN) { + apr_pool_clear(ptry); apr_sleep(apr_time_from_msec(H3_PORT_ACQUIRE_RETRY_MS)); continue; } @@ -65,7 +73,7 @@ static void* APR_THREAD_FUNC port_acquire_thread_fn(apr_thread_t* thread H3_UNUS h3_socket_close(udp_fd); break; } - if (h3_io_listen_start(args->pchild, args->vhost, args->conf, udp_fd) != APR_SUCCESS) + if (h3_io_listen_start(args->pool, args->vhost, args->conf, udp_fd) != APR_SUCCESS) { ap_log_error(APLOG_MARK, APLOG_ERR, 0, args->vhost, "port acquirer: h3_io_listen_start failed"); h3_socket_close(udp_fd); @@ -96,6 +104,47 @@ static h3_server_conf* find_h3_server(server_rec* s, server_rec** out_server) return conf; } +static void stop_child_threads(int graceful); + +/* mod_http3 allocates from its own threads, so it gets a pool of its own + * whose allocator has a mutex. The child pool's allocator may have none: + * prefork shares it, unlocked, with the pool of every request. */ +static apr_status_t create_h3_pool(apr_pool_t* pchild, apr_pool_t** out) +{ + apr_allocator_t* allocator = NULL; + apr_thread_mutex_t* mutex = NULL; + apr_status_t rv = apr_allocator_create(&allocator); + if (rv != APR_SUCCESS) + { + return rv; + } + rv = apr_pool_create_ex(out, pchild, NULL, allocator); + if (rv != APR_SUCCESS) + { + apr_allocator_destroy(allocator); + return rv; + } + apr_allocator_owner_set(allocator, *out); + apr_pool_tag(*out, "h3_child"); + rv = apr_thread_mutex_create(&mutex, APR_THREAD_MUTEX_DEFAULT, *out); + if (rv != APR_SUCCESS) + { + return rv; + } + apr_allocator_mutex_set(allocator, mutex); + return APR_SUCCESS; +} + +/* The threads must be gone before the child pool they run from is + * destroyed, and before exit() runs OpenSSL's cleanup under them. Most + * MPMs stop them through the child_stopping hook, but prefork skips that + * hook when a child exits from a signal handler. */ +static apr_status_t child_pool_cleanup(void* data H3_UNUSED) +{ + stop_child_threads(0); + return APR_SUCCESS; +} + void h3_child_init(apr_pool_t* pchild, server_rec* s) { server_rec* vhost = NULL; @@ -105,18 +154,25 @@ void h3_child_init(apr_pool_t* pchild, server_rec* s) ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, "h3_child_init: no H3 cert/key configured, skipping"); return; } + apr_pool_pre_cleanup_register(pchild, NULL, child_pool_cleanup); + apr_pool_t* pool = NULL; + if (create_h3_pool(pchild, &pool) != APR_SUCCESS) + { + ap_log_error(APLOG_MARK, APLOG_ERR, 0, vhost, APLOGNO() "h3_child_init: could not create the HTTP/3 pool"); + return; + } int udp_fd = -1; - apr_status_t rv = h3_socket_open(conf->h3_port, conf->h3_socket_buffer_size, pchild, &udp_fd); + apr_status_t rv = h3_socket_open(conf->h3_port, conf->h3_socket_buffer_size, pool, &udp_fd); if (rv == APR_EAGAIN) { ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, vhost, "h3_child_init: pid=%d port already owned, will keep retrying in background", h3_getpid()); - struct port_acquire_args* args = apr_palloc(pchild, sizeof(*args)); - args->pchild = pchild; + struct port_acquire_args* args = apr_palloc(pool, sizeof(*args)); + args->pool = pool; args->vhost = vhost; args->conf = conf; apr_threadattr_t* attr = NULL; - if (apr_threadattr_create(&attr, pchild) != APR_SUCCESS || apr_thread_create(&port_acquire_thread, attr, port_acquire_thread_fn, args, pchild) != APR_SUCCESS) + if (apr_threadattr_create(&attr, pool) != APR_SUCCESS || apr_thread_create(&port_acquire_thread, attr, port_acquire_thread_fn, args, pool) != APR_SUCCESS) { ap_log_error(APLOG_MARK, APLOG_ERR, 0, vhost, "h3_child_init: failed to start port acquirer thread"); port_acquire_thread = NULL; @@ -128,16 +184,15 @@ void h3_child_init(apr_pool_t* pchild, server_rec* s) ap_log_error(APLOG_MARK, APLOG_ERR, 0, vhost, "h3_child_init: h3_socket_open failed"); return; } - if (h3_io_listen_start(pchild, vhost, conf, udp_fd) != APR_SUCCESS) + if (h3_io_listen_start(pool, vhost, conf, udp_fd) != APR_SUCCESS) { ap_log_error(APLOG_MARK, APLOG_ERR, 0, vhost, "h3_child_init: h3_io_listen_start failed"); h3_socket_close(udp_fd); } } -void h3_c1_child_stopping(apr_pool_t* p H3_UNUSED, int graceful) +static void stop_child_threads(int graceful) { - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, NULL, "mod_http3: child stopping (graceful=%d)", graceful); child_stopping = 1; if (port_acquire_thread) { @@ -153,6 +208,12 @@ void h3_c1_child_stopping(apr_pool_t* p H3_UNUSED, int graceful) h3_io_listen_stop(child_h3_io); } +void h3_c1_child_stopping(apr_pool_t* p H3_UNUSED, int graceful) +{ + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, NULL, "mod_http3: child stopping (graceful=%d)", graceful); + stop_child_threads(graceful); +} + #if H3_DEVEL void h3_c1_child_stopped(apr_pool_t* p H3_UNUSED, int graceful) From 5856eeebd56441b542984e741b592baefd2342c9 Mon Sep 17 00:00:00 2001 From: "Michiel W. Beijen" Date: Wed, 4 Nov 2026 19:39:12 +0100 Subject: [PATCH 2/3] mod_http3: Move the thread-safe pool helper to h3_threads Review feedback: keep the body of stop_child_threads() in h3_c1_child_stopping(), which the child pool cleanup now calls, so no forward declaration is needed. Move the pool creation to h3_threads.c so other code can reuse it. * modules/http3/h3_server.c (h3_c1_child_stopping): Fold in stop_child_threads(). * modules/http3/h3_threads.c, modules/http3/include/h3_threads.h (h3_thread_safe_pool_create): New, from create_h3_pool() in h3_server.c. Generated-by: Claude Code (Claude Opus 5.5) --- modules/http3/h3_server.c | 45 ++++-------------------------- modules/http3/h3_threads.c | 28 +++++++++++++++++++ modules/http3/include/h3_threads.h | 14 ++++++++++ 3 files changed, 47 insertions(+), 40 deletions(-) diff --git a/modules/http3/h3_server.c b/modules/http3/h3_server.c index 4c74ef66b49..3de6360be06 100644 --- a/modules/http3/h3_server.c +++ b/modules/http3/h3_server.c @@ -30,6 +30,7 @@ #include "h3_os.h" #include "h3_server.h" #include "h3_socket.h" +#include "h3_threads.h" #include "mod_http3.h" static volatile int child_stopping = 0; @@ -104,44 +105,13 @@ static h3_server_conf* find_h3_server(server_rec* s, server_rec** out_server) return conf; } -static void stop_child_threads(int graceful); - -/* mod_http3 allocates from its own threads, so it gets a pool of its own - * whose allocator has a mutex. The child pool's allocator may have none: - * prefork shares it, unlocked, with the pool of every request. */ -static apr_status_t create_h3_pool(apr_pool_t* pchild, apr_pool_t** out) -{ - apr_allocator_t* allocator = NULL; - apr_thread_mutex_t* mutex = NULL; - apr_status_t rv = apr_allocator_create(&allocator); - if (rv != APR_SUCCESS) - { - return rv; - } - rv = apr_pool_create_ex(out, pchild, NULL, allocator); - if (rv != APR_SUCCESS) - { - apr_allocator_destroy(allocator); - return rv; - } - apr_allocator_owner_set(allocator, *out); - apr_pool_tag(*out, "h3_child"); - rv = apr_thread_mutex_create(&mutex, APR_THREAD_MUTEX_DEFAULT, *out); - if (rv != APR_SUCCESS) - { - return rv; - } - apr_allocator_mutex_set(allocator, mutex); - return APR_SUCCESS; -} - /* The threads must be gone before the child pool they run from is * destroyed, and before exit() runs OpenSSL's cleanup under them. Most * MPMs stop them through the child_stopping hook, but prefork skips that * hook when a child exits from a signal handler. */ static apr_status_t child_pool_cleanup(void* data H3_UNUSED) { - stop_child_threads(0); + h3_c1_child_stopping(NULL, 0); return APR_SUCCESS; } @@ -156,7 +126,7 @@ void h3_child_init(apr_pool_t* pchild, server_rec* s) } apr_pool_pre_cleanup_register(pchild, NULL, child_pool_cleanup); apr_pool_t* pool = NULL; - if (create_h3_pool(pchild, &pool) != APR_SUCCESS) + if (h3_thread_safe_pool_create(pchild, "h3_child", &pool) != APR_SUCCESS) { ap_log_error(APLOG_MARK, APLOG_ERR, 0, vhost, APLOGNO() "h3_child_init: could not create the HTTP/3 pool"); return; @@ -191,8 +161,9 @@ void h3_child_init(apr_pool_t* pchild, server_rec* s) } } -static void stop_child_threads(int graceful) +void h3_c1_child_stopping(apr_pool_t* p H3_UNUSED, int graceful) { + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, NULL, "mod_http3: child stopping (graceful=%d)", graceful); child_stopping = 1; if (port_acquire_thread) { @@ -208,12 +179,6 @@ static void stop_child_threads(int graceful) h3_io_listen_stop(child_h3_io); } -void h3_c1_child_stopping(apr_pool_t* p H3_UNUSED, int graceful) -{ - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, NULL, "mod_http3: child stopping (graceful=%d)", graceful); - stop_child_threads(graceful); -} - #if H3_DEVEL void h3_c1_child_stopped(apr_pool_t* p H3_UNUSED, int graceful) diff --git a/modules/http3/h3_threads.c b/modules/http3/h3_threads.c index 7d186e0fa39..e309c04f441 100644 --- a/modules/http3/h3_threads.c +++ b/modules/http3/h3_threads.c @@ -21,6 +21,8 @@ #include #include +#include +#include #include "h3.h" #include "h3_io.h" @@ -32,6 +34,32 @@ #define H3_PUMP_BUDGET 32 +apr_status_t h3_thread_safe_pool_create(apr_pool_t* parent, const char* tag, apr_pool_t** out) +{ + apr_allocator_t* allocator = NULL; + apr_thread_mutex_t* mutex = NULL; + apr_status_t rv = apr_allocator_create(&allocator); + if (rv != APR_SUCCESS) + { + return rv; + } + rv = apr_pool_create_ex(out, parent, NULL, allocator); + if (rv != APR_SUCCESS) + { + apr_allocator_destroy(allocator); + return rv; + } + apr_allocator_owner_set(allocator, *out); + apr_pool_tag(*out, tag); + rv = apr_thread_mutex_create(&mutex, APR_THREAD_MUTEX_DEFAULT, *out); + if (rv != APR_SUCCESS) + { + return rv; + } + apr_allocator_mutex_set(allocator, mutex); + return APR_SUCCESS; +} + void* APR_THREAD_FUNC h3_event_thread(apr_thread_t* thread, void* data) { (void)thread; diff --git a/modules/http3/include/h3_threads.h b/modules/http3/include/h3_threads.h index 90de5541a6f..b594add38b9 100644 --- a/modules/http3/include/h3_threads.h +++ b/modules/http3/include/h3_threads.h @@ -18,6 +18,7 @@ #define H3_THREADS_H #include "h3_io.h" +#include #include /** @@ -51,4 +52,17 @@ void* APR_THREAD_FUNC worker_thread(apr_thread_t* thread, void* data); */ void* APR_THREAD_FUNC h3_event_thread(apr_thread_t* thread, void* data); +/** + * Create a pool that may be allocated from by more than one thread: it has + * an allocator of its own, with a mutex. Pools such as the child pool may + * have an allocator without one; prefork shares it, unlocked, with the + * pool of every request. + * + * @param parent The parent pool. + * @param tag The tag for the new pool. + * @param out Receives the new pool. + * @return APR_SUCCESS, or the error from APR. + */ +apr_status_t h3_thread_safe_pool_create(apr_pool_t* parent, const char* tag, apr_pool_t** out); + #endif /* H3_THREADS_H */ From 4325ba9224539705bb5d5616c6c86212f1505ae6 Mon Sep 17 00:00:00 2001 From: "Michiel W. Beijen" Date: Wed, 4 Nov 2026 19:53:20 +0100 Subject: [PATCH 3/3] mod_http3: Do not allocate from the HTTP/3 pool from two threads Review feedback: the mutex on the allocator of the HTTP/3 pool covers subpools, not allocations from the pool itself. h3_io_listen_start() registered its pool cleanup after starting the event thread, which allocates from the same pool, so the two could allocate at once when the port acquirer thread started listening. Register it first. Also destroy the pool when its mutex cannot be created, log when the port acquirer cannot create its pool, drop an empty APLOGNO() that no other message in the file has, and keep the declarations in h3_child_init() at the top of their blocks, as C89 requires. * modules/http3/h3_io.c (h3_io_listen_start): Register the cleanup before starting the event thread. * modules/http3/h3_server.c (port_acquire_thread_fn): Log the pool creation failure. (h3_child_init): Declare variables first; drop the APLOGNO(). * modules/http3/h3_threads.c (h3_thread_safe_pool_create): Destroy the pool if its mutex cannot be created. * modules/http3/include/h3_threads.h: Say what the mutex does not cover. Generated-by: Claude Code (Claude Opus 5.5) --- modules/http3/h3_io.c | 4 +++- modules/http3/h3_server.c | 14 ++++++++------ modules/http3/h3_threads.c | 2 ++ modules/http3/include/h3_threads.h | 10 ++++++---- 4 files changed, 19 insertions(+), 11 deletions(-) diff --git a/modules/http3/h3_io.c b/modules/http3/h3_io.c index 22f618b98ec..ef9c3177fba 100644 --- a/modules/http3/h3_io.c +++ b/modules/http3/h3_io.c @@ -155,7 +155,10 @@ apr_status_t h3_io_listen_start(apr_pool_t* pchild, server_rec* s, h3_server_con io->thread_running = 1; child_h3_io = io; + /* The event thread allocates from pchild too, so allocate everything + * from it here before the thread starts. */ apr_threadattr_t* attr = NULL; + apr_pool_pre_cleanup_register(pchild, io, listen_stop_cleanup); if (apr_threadattr_create(&attr, pchild) != APR_SUCCESS || apr_thread_create(&io->event_thread, attr, h3_event_thread, io, pchild) != APR_SUCCESS) { io->thread_running = 0; @@ -163,7 +166,6 @@ apr_status_t h3_io_listen_start(apr_pool_t* pchild, server_rec* s, h3_server_con child_h3_io = NULL; return APR_EGENERAL; } - apr_pool_pre_cleanup_register(pchild, io, listen_stop_cleanup); ap_log_error(APLOG_MARK, APLOG_INFO, 0, s, "mod_http3 loaded with version: %d (%s) on pid=%d port=%d; built against httpd %d.%d.%d MMN %d:%d (%s)", MOD_HTTP3_VERSION, MOD_HTTP3_VERSION_STRING, h3_getpid(), (int)conf->h3_port, AP_SERVER_MAJORVERSION_NUMBER, AP_SERVER_MINORVERSION_NUMBER, AP_SERVER_PATCHLEVEL_NUMBER, MODULE_MAGIC_NUMBER_MAJOR, MODULE_MAGIC_NUMBER_MINOR, H3_DEVEL ? "devel" : "stable"); return APR_SUCCESS; diff --git a/modules/http3/h3_server.c b/modules/http3/h3_server.c index 3de6360be06..df2ecf3dfe4 100644 --- a/modules/http3/h3_server.c +++ b/modules/http3/h3_server.c @@ -52,6 +52,7 @@ static void* APR_THREAD_FUNC port_acquire_thread_fn(apr_thread_t* thread H3_UNUS apr_pool_t* ptry = NULL; if (apr_pool_create(&ptry, args->pool) != APR_SUCCESS) { + ap_log_error(APLOG_MARK, APLOG_ERR, 0, args->vhost, "port acquirer: could not create a pool"); return NULL; } while (!child_stopping) @@ -119,29 +120,30 @@ void h3_child_init(apr_pool_t* pchild, server_rec* s) { server_rec* vhost = NULL; h3_server_conf* conf = find_h3_server(s, &vhost); + apr_pool_t* pool = NULL; + int udp_fd = -1; + apr_status_t rv; if (!conf) { ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, "h3_child_init: no H3 cert/key configured, skipping"); return; } apr_pool_pre_cleanup_register(pchild, NULL, child_pool_cleanup); - apr_pool_t* pool = NULL; if (h3_thread_safe_pool_create(pchild, "h3_child", &pool) != APR_SUCCESS) { - ap_log_error(APLOG_MARK, APLOG_ERR, 0, vhost, APLOGNO() "h3_child_init: could not create the HTTP/3 pool"); + ap_log_error(APLOG_MARK, APLOG_ERR, 0, vhost, "h3_child_init: could not create the HTTP/3 pool"); return; } - int udp_fd = -1; - apr_status_t rv = h3_socket_open(conf->h3_port, conf->h3_socket_buffer_size, pool, &udp_fd); + rv = h3_socket_open(conf->h3_port, conf->h3_socket_buffer_size, pool, &udp_fd); if (rv == APR_EAGAIN) { - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, vhost, "h3_child_init: pid=%d port already owned, will keep retrying in background", h3_getpid()); struct port_acquire_args* args = apr_palloc(pool, sizeof(*args)); + apr_threadattr_t* attr = NULL; + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, vhost, "h3_child_init: pid=%d port already owned, will keep retrying in background", h3_getpid()); args->pool = pool; args->vhost = vhost; args->conf = conf; - apr_threadattr_t* attr = NULL; if (apr_threadattr_create(&attr, pool) != APR_SUCCESS || apr_thread_create(&port_acquire_thread, attr, port_acquire_thread_fn, args, pool) != APR_SUCCESS) { ap_log_error(APLOG_MARK, APLOG_ERR, 0, vhost, "h3_child_init: failed to start port acquirer thread"); diff --git a/modules/http3/h3_threads.c b/modules/http3/h3_threads.c index e309c04f441..1939fd76349 100644 --- a/modules/http3/h3_threads.c +++ b/modules/http3/h3_threads.c @@ -54,6 +54,8 @@ apr_status_t h3_thread_safe_pool_create(apr_pool_t* parent, const char* tag, apr rv = apr_thread_mutex_create(&mutex, APR_THREAD_MUTEX_DEFAULT, *out); if (rv != APR_SUCCESS) { + apr_pool_destroy(*out); + *out = NULL; return rv; } apr_allocator_mutex_set(allocator, mutex); diff --git a/modules/http3/include/h3_threads.h b/modules/http3/include/h3_threads.h index b594add38b9..c057d66c5b6 100644 --- a/modules/http3/include/h3_threads.h +++ b/modules/http3/include/h3_threads.h @@ -53,10 +53,12 @@ void* APR_THREAD_FUNC worker_thread(apr_thread_t* thread, void* data); void* APR_THREAD_FUNC h3_event_thread(apr_thread_t* thread, void* data); /** - * Create a pool that may be allocated from by more than one thread: it has - * an allocator of its own, with a mutex. Pools such as the child pool may - * have an allocator without one; prefork shares it, unlocked, with the - * pool of every request. + * Create a pool with an allocator of its own, which has a mutex, so that + * threads can create and destroy subpools of it concurrently. Pools such + * as the child pool may have an allocator without one; prefork shares it, + * unlocked, with the pool of every request. The mutex does not make + * allocating from the pool itself thread-safe: only one thread at a time + * may do that. * * @param parent The parent pool. * @param tag The tag for the new pool.