mod_http3: Keep the child's threads off the child pool

modules/http3/h3_io.c

@@ -155,15 +155,17 @@ 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;
         teardown(io);
         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;

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;
@@ -38,20 +39,29 @@ 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;
 };
 
 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)
+    {
+        ap_log_error(APLOG_MARK, APLOG_ERR, 0, args->vhost, "port acquirer: could not create a pool");
+        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 +75,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,27 +106,45 @@ static h3_server_conf* find_h3_server(server_rec* s, server_rec** out_server)
     return conf;
 }
 
+/* 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)
+{
+    h3_c1_child_stopping(NULL, 0);
+    return APR_SUCCESS;
+}
+
 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);
+    if (h3_thread_safe_pool_create(pchild, "h3_child", &pool) != APR_SUCCESS)
+    {
+        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, pchild, &udp_fd);
+    rv = h3_socket_open(conf->h3_port, conf->h3_socket_buffer_size, pool, &udp_fd);
     if (rv == APR_EAGAIN)
     {
+        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());
-        struct port_acquire_args* args = apr_palloc(pchild, sizeof(*args));
-        args->pchild = pchild;
+        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,7 +156,7 @@ 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);

modules/http3/h3_threads.c

@@ -21,6 +21,8 @@
 
 #include <apr_atomic.h>
 #include <apr_hash.h>
+#include <apr_pools.h>
+#include <apr_thread_mutex.h>
 
 #include "h3.h"
 #include "h3_io.h"
@@ -32,6 +34,34 @@
 
 #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)
+    {
+        apr_pool_destroy(*out);
+        *out = NULL;
+        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;

modules/http3/include/h3_threads.h

@@ -18,6 +18,7 @@
 #define H3_THREADS_H
 
 #include "h3_io.h"
+#include <apr_pools.h>
 #include <apr_thread_proc.h>
 
 /**
@@ -51,4 +52,19 @@ 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 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.
+ * @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 */