HTTP/3 improvements, threading fixes, ngtcp2 example code removal - #8
Merged
Merged
Conversation
It was an unmodified copy of the ngtcp2 upstream simple client example and the only thing in the project requiring /opt/boringssl. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This reverts commit bf34a2c.
udp_on_read() collected the sessions that received something in this batch in a vector, de-duplicating with a linear std::ranges::find on every packet. boost::container::small_flat_set does the same lookup on sorted storage and keeps the first 32 entries inline, so the common batch still costs no allocation. The write pass now visits sessions in pointer order instead of arrival order, which does not matter: each session is flushed independently. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
"session created" and "closing/draining period over" fire once per QUIC session and are only interesting when tracing, so they move to debug. The TCP listen message now says which socket it is, matching the UDP one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Http3IdleTimeout's server thread called context.run() directly, so it was the one place that lost the per-handler trace the ::run() helper prints in debug builds. Also reflows two comment lines in client_impl_udp.cpp. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The vendored copy of ngtcp2's examples/ directory was carried along mainly for
the ngtcp-client/ngtcp-server binaries. The devcontainer base image now builds
the upstream examples and installs them as osslclient/osslserver, so the copy
has no reason to exist any more.
Keep only what the library actually calls:
network.h sockaddr_union, Address
util.{h,cc} format_hex(), timestamp(), straddr()
shared.{h,cc} msghdr_get_local_addr(), set_port()
Everything else goes: both examples and their bases, all tls_*, debug, http,
siphash, util_openssl, util_test, template.h, and the ngtcp2_common static lib
along with the subdirectory CMakeLists.txt. The two remaining .cc files are
compiled straight into the anyhttp target, so the HAVE_* header checks in the
top-level CMakeLists.txt (never consumed by anything) are gone too and the
kept sources include the system headers unconditionally.
literals.hpp defines _k/_m itself now instead of pulling in template.h, and
HTTP3.md points at osslserver for manual testing.
The devcontainer side of this: the base image builds ngtcp2's examples and
installs osslclient/osslserver by hand (they are noinst_PROGRAMS), and the
devcontainer itself is pinned to the psedoc/anyhttp:testing image carrying
them.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Running the server with multiple threads crashed immediately under load (ngtcp2 assertions "conn->log.last_ts <= ts" and PPE_PENDING): timers, reads and writes touched the same ngtcp2_conn from different threads. The serialization unit is the QUIC connection, not the CID — many CIDs (client DCID, minted SCIDs) alias one ngtcp2_conn. Each Http3Session now owns a strand (gated on Config::use_strand, like TCP), so its timers, write flushes and request-handler coroutines serialize automatically. udp_receive_loop() stays a single demux coroutine: it copies datagrams, groups each receive batch by session and posts one QuicBatch per session to that session's strand (process_quic_batch), keeping the one-flush- per-batch GSO aggregation. A new connection's DCID is published to the table immediately so retransmitted Initials queue behind init(). The CID table is now guarded by m_quicMutex. Sends need no serialization (sendto is atomic per datagram), but each session dup()s the UDP fd so the final CONNECTION_CLOSE can't race the server closing its socket. TSAN surfaced two shutdown races, both fixed: - ~Http3Session runs wherever the last shared_ptr drops; stream teardown moved to do_destroy() on the session's executor so the destructor has no strand-visible state left. - destroy() closed the UDP socket from a foreign thread while the receive loop re-armed async_wait; the socket now lives on its own strand and close() is dispatched through it. Verified with h2load at -t 20 (10k and 100k requests), kill-under-load, 5 clean TSAN rounds, and the full test suite (195/195). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ExternalTLSThreaded runs the io_context on 8 threads, so the server puts every connection on its own strand, and hits it with h2load (1000 requests, 8 connections, 5 concurrent streams each) for HTTP/1.1, HTTP/2 and HTTP/3. Reverting the previous commit makes the HTTP/3 case abort on the ngtcp2 assertions it fixed. The thread count is now a virtual hook on the Server fixture (replacing the MULTITHREADED ifdefs, which still set the default), and the h2load body moved into a helper so both fixtures share it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The 65535 was a second, silent copy of the size of test/data/64kminus1; take it from the file itself, like testFile/testFileSize does. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
BOOST_PROCESS_USE_STD_FS is defined project-wide, so bp::filesystem already *is* std::filesystem; spell it that way and include <filesystem> instead of <boost/filesystem/path.hpp>. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The server runs a TCP accept loop and a UDP receive loop side by side, and "accept: Operation canceled" alone doesn't say which one just went away. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Handing a response chunk or EOF to the application resumes its coroutine, which may drop the last reference to the Session right there -- so close() can run from inside on_read(), in the middle of the receive loop's body. Its socket_.cancel() then finds no receive pending, because we are between two of them, and nothing keeps the loop from arming one more that no peer will ever complete: the server, already draining because it got our CONNECTION_CLOSE, does not even answer it. The io_context is left with a work count it can never drop. Single-threaded this went unnoticed: the server's own teardown packet reliably arrived first, so on_read() failed and broke the loop. With the server tearing down on its own strand, that packet never comes, and every HTTP/3 client test hung. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Boost.Process v2's posix launcher calls execution_context::notify_fork() in the child, between fork() and execve(). That walks the io_context's services and locks their mutexes -- and with more than one thread running the context, another thread may hold one of them at the moment of the fork, so the child inherits it locked and sleeps on it forever, before it ever gets to dup2() its stdio or exec. The externally spawned curl/h2load tests then wait on pipes that never reach EOF. The child does nothing but exec, so it has no use for the notification. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two threads, which is enough to put the server's per-connection strands and the client's strand on different threads. Two testcases needed fixing to survive that: ResetServerDuringRequest joined its spawned send() with future.get(), which blocks the very strand whose handlers would complete that send. Awaiting an asio::experimental::promise starts the coroutine just as eagerly, without occupying the thread. WHEN_send_more_than_content_length_THEN_connection_is_reset insisted on ECONNRESET, but which of ECONNRESET and EPIPE the write reports is a matter of how far the kernel has gotten with the peer's RST when it lands. Both mean the same thing here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Add anyhttp::tls_handshake_info(), which condenses the facts h2load prints after a handshake -- protocol version, cipher, key exchange group and the negotiated ALPN -- into a single line, so the log looks the same no matter which transport a session runs on. The TCP server had no handshake log at all; the two HTTP/3 sessions only said "TLS handshake complete". All three now use the shared summary. The group name comes from SSL_get0_group_name() first: the EVP_PKEY route h2load uses has no name for the post-quantum hybrids that are the default in OpenSSL 3.5, and reports them as "unknown". The peer temp key is still used for the bit count, with the EVP_PKEY route as fallback. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.