diff --git a/changes-entries/h3_blocked_stream_spin.txt b/changes-entries/h3_blocked_stream_spin.txt new file mode 100644 index 00000000000..1b230bfa5d4 --- /dev/null +++ b/changes-entries/h3_blocked_stream_spin.txt @@ -0,0 +1,4 @@ + *) mod_http3: Fixed an event-thread spin while a client holds its + flow-control window shut: a blocked stream is now blocked again each time + nghttp3 offers it. + [Alexander Gerasimov ] diff --git a/changes-entries/h3_drain_pending_handshakes.txt b/changes-entries/h3_drain_pending_handshakes.txt new file mode 100644 index 00000000000..c38c37f7041 --- /dev/null +++ b/changes-entries/h3_drain_pending_handshakes.txt @@ -0,0 +1,4 @@ + *) mod_http3: Fixed a graceful restart that never ended: a handshake still + pending when the child started to drain did not finish or time out, so + the old child kept its connection count and the port. + [Alexander Gerasimov ] diff --git a/changes-entries/h3_session_allocator_race.txt b/changes-entries/h3_session_allocator_race.txt new file mode 100644 index 00000000000..87ebb36e22b --- /dev/null +++ b/changes-entries/h3_session_allocator_race.txt @@ -0,0 +1,3 @@ + *) mod_http3: Fixed a heap race that crashed the child: worker threads and + the event thread allocated from one session allocator that had no mutex. + [Alexander Gerasimov ] diff --git a/modules/http3/h3_io.c b/modules/http3/h3_io.c index e6d6d56ffef..78bcb4cefe8 100644 --- a/modules/http3/h3_io.c +++ b/modules/http3/h3_io.c @@ -285,6 +285,15 @@ static apr_status_t spawn_serviced_session(h3_io_t* io, h3q_conn* conn) } } apr_pool_tag(session_pool, "h3_session"); + /* Workers allocate from stream subpools while this thread makes its own: share the allocator safely. */ + apr_thread_mutex_t* alloc_mutex = NULL; + if (apr_thread_mutex_create(&alloc_mutex, APR_THREAD_MUTEX_DEFAULT, session_pool) != APR_SUCCESS) + { + apr_pool_destroy(session_pool); + h3q_conn_free(conn); + return APR_EGENERAL; + } + apr_allocator_mutex_set(allocator, alloc_mutex); h3_session* session = NULL; if (h3_session_create(&session, io->server, conn, session_pool) != APR_SUCCESS) diff --git a/modules/http3/h3_server.c b/modules/http3/h3_server.c index 176f1fb95e1..0b124a61628 100644 --- a/modules/http3/h3_server.c +++ b/modules/http3/h3_server.c @@ -54,6 +54,7 @@ static void* APR_THREAD_FUNC port_acquire_thread_fn(apr_thread_t* thread H3_UNUS 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"); + apr_atomic_set32(&port_acquire_running, 0); return NULL; } while (!child_stopping) diff --git a/modules/http3/h3_stream.c b/modules/http3/h3_stream.c index d5bc2772562..45b9dc68226 100644 --- a/modules/http3/h3_stream.c +++ b/modules/http3/h3_stream.c @@ -126,13 +126,14 @@ void flush_nghttp3(h3_session* session) nghttp3_conn_add_ack_offset(session->ngh3, sid, res.accepted); if (res.blocked) { - /* Send buffer full: skip this stream instead of busy-looping on the same vec. */ + /* Send buffer full: skip this stream instead of busy-looping on the same vec. + * Block it again every time: resume_stream reschedules a blocked stream. */ if (!h3s->write_blocked) { h3s->write_blocked = 1; session->blocked_streams++; - nghttp3_conn_block_stream(session->ngh3, sid); } + nghttp3_conn_block_stream(session->ngh3, sid); continue; } } diff --git a/modules/http3/h3_threads.c b/modules/http3/h3_threads.c index 4a7e40fcb37..7547cbe828b 100644 --- a/modules/http3/h3_threads.c +++ b/modules/http3/h3_threads.c @@ -126,6 +126,10 @@ void* APR_THREAD_FUNC h3_event_thread(apr_thread_t* thread, void* data) h3q_conn_free(conn); } } + } + if (io->thread_running) + { + /* Also while draining: a pending handshake counts as an MPM connection, so it must finish or time out. */ progress_pending_handshakes(io); } diff --git a/test/modules/http3/test_014_lifecycle.py b/test/modules/http3/test_014_lifecycle.py index 31c04591dc8..c23e3c20482 100644 --- a/test/modules/http3/test_014_lifecycle.py +++ b/test/modules/http3/test_014_lifecycle.py @@ -43,9 +43,14 @@ def test_h3_014_04_stop_cleanly(self, env): H3Conf(env).add_h3_vhost().install() log = env.httpd_error_log start = log.current_pos() + url = env.mkurl("https", "test1", "/001.html") for _ in range(20): assert env.apache_restart() == 0 assert env.wait_udp_port(env.https_port) + # a live session, so stop also tears down its control streams + r = env.h3_get(url) + assert r.exit_code == 0, f"{r}" + assert r.response["protocol"] == "HTTP/3", f"{r}" assert env.apache_stop() == 0 # crashed, or still there when the parent gave up waiting unclean = [log.RE_ERRLOG_CRASH.pattern, r'.*AH0004[56]: child process']