diff --git a/README.md b/README.md index e6201b5..59c8d59 100644 --- a/README.md +++ b/README.md @@ -275,6 +275,7 @@ The library includes tests covering: - HTTP/3 external interop (`ShouldSend`) — live fetch from `cloudflare-quic.com:443`, exercises real TLS chain validation, mandatory control stream, peer-initiated unidi streams, and QPACK Huffman decoding - QUIC reliable streams (`ShouldSendRecieveQUICStream`) - QUIC unreliable datagrams (`ShouldSendRecieveQUICDatagram`) +- QUIC connection teardown (`ShouldSurviveConnectionChurn`) — 25 build-up/tear-down cycles with reader threads parked inside `RecieveSync`, the shape a reconnect loop has; the rest of the QUIC tests end in `std::_Exit(0)` and so never run a `ClientQUIC` destructor - WebTransport echo (`ShouldEchoWebTransport`) — extended-CONNECT acceptance, draft-02 SETTINGS, bidi data stream framing (`WT_STREAM 0x41` + session-id varint), and byte-for-byte echo - HTTP/1.1 wire format (`ShouldParseHTTP1`) — serialisation, incremental parsing fed one byte at a time, chunked bodies with trailers, pipelining, interim 1xx, HTTP/1.0 and HEAD framing, and the malformed inputs the parser is required to reject - HTTP/1.1 round-trip (`ShouldSendRecieveHTTP1`) — routing, query strings, HEAD, 404 and a throwing handler diff --git a/project.cpp b/project.cpp index 59fd6ec..0d6faa1 100644 --- a/project.cpp +++ b/project.cpp @@ -149,6 +149,7 @@ extern "C" Configuration CrafterBuildProject(std::span a cfg.AddTest("ShouldSendRecieveQUICDatagram").Dependencies({ &cfg }); cfg.AddTest("ShouldSendRecieveQUICStream").Dependencies({ &cfg }); cfg.AddTest("ShouldSurviveAbuseHTTP1").Dependencies({ &cfg }); + cfg.AddTest("ShouldSurviveConnectionChurn").Dependencies({ &cfg }); } return cfg; diff --git a/tests/ShouldSurviveConnectionChurn/main.cpp b/tests/ShouldSurviveConnectionChurn/main.cpp new file mode 100644 index 0000000..ca0b54a --- /dev/null +++ b/tests/ShouldSurviveConnectionChurn/main.cpp @@ -0,0 +1,136 @@ +//SPDX-License-Identifier: LGPL-3.0-only +//SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts® + +// Regression test for the connection-teardown race in ~ClientQUIC. +// +// QUICStream::Stop only *initiates* a graceful shutdown; the actual +// MsQuicStreamClose happens later, from the terminal SHUTDOWN_COMPLETE +// callback on an msquic worker thread. ~ClientQUIC used to call +// MsQuicConnectionClose straight away, so a connection could be closed while +// streams belonging to it were still open on the msquic side — which trips an +// msquic bugcheck and aborts the process. +// +// Nothing else in the suite exercises this: every other QUIC test ends in +// std::_Exit(0) and so never runs a ClientQUIC destructor. The shape that +// breaks is a reconnect loop — build a connection, tear it down, build +// another — which is exactly what this test does, `cycles` times. +// +// The blocked readers matter. Each inbound stream gets a thread parked inside +// RecieveSync for the life of the connection (the shape a long-lived +// streaming client has), so teardown happens with readers still inside the +// receive path. An otherwise identical loop whose handler reads once and +// returns does not reproduce. + +import Crafter.Network; +import Crafter.Thread; +import std; +using namespace Crafter; + +namespace { + constexpr int cycles = 25; + constexpr int streamsPerConnection = 3; +} + +int main() { + ThreadPool::Start(); + + // On a build where a teardown deadlocks rather than aborts, fail the test + // instead of hanging the whole suite. + std::thread watchdog([] { + std::this_thread::sleep_for(std::chrono::seconds(90)); + std::println("timed out — a connection teardown never completed"); + std::cout.flush(); + std::_Exit(1); + }); + watchdog.detach(); + + for (int i = 0; i < cycles; ++i) { + const std::uint16_t port = static_cast(19200 + i); + const std::string alpn = "churn/1"; + + QUICServerCredentials serverCreds; + serverCreds.selfSigned = true; + + std::mutex serverMutex; + std::vector serverStreams; + ClientQUIC* accepted = nullptr; + + auto listener = std::make_unique( + port, alpn, serverCreds, [&](ClientQUIC* peer) { + std::lock_guard lock(serverMutex); + accepted = peer; + for (int s = 0; s < streamsPerConnection; ++s) { + QUICStream stream = peer->OpenStream(/*unidirectional=*/true); + const char payload[] = "hello"; + stream.SendSync(payload, sizeof(payload) - 1, /*finish=*/false); + serverStreams.push_back(std::move(stream)); + } + }); + listener->ListenAsyncAsync(); + + QUICClientCredentials clientCreds; + clientCreds.insecureNoServerValidation = true; + + { + ClientQUIC client(std::string("localhost"), port, alpn, clientCreds); + + std::mutex clientMutex; + std::condition_variable clientCv; + std::vector readers; + int started = 0; + + client.OnStream([&](QUICStream stream) { + auto shared = std::make_shared(std::move(stream)); + std::thread reader([shared] { + try { + while (true) (void)shared->RecieveSync(); + } catch (...) { + // Connection closed. + } + }); + std::lock_guard lock(clientMutex); + readers.push_back(std::move(reader)); + ++started; + clientCv.notify_all(); + }); + + { + std::unique_lock lock(clientMutex); + clientCv.wait_for(lock, std::chrono::seconds(5), [&] { + return started == streamsPerConnection; + }); + if (started != streamsPerConnection) { + std::println("cycle {}: only {} of {} streams arrived", + i, started, streamsPerConnection); + return 1; + } + } + + client.Stop(); + for (std::thread& reader : readers) { + if (reader.joinable()) reader.join(); + } + // `client` is destroyed here, once the readers have unwound. + } + + { + std::lock_guard lock(serverMutex); + // Deliberately the other order from the client side: destroy the + // connection while the app still holds its stream wrappers, so + // ~ClientQUIC has to cope with streams that are open as far as + // msquic is concerned. + delete accepted; + accepted = nullptr; + serverStreams.clear(); + } + listener->Stop(); + listener.reset(); + } + + std::println("survived {} connection teardowns", cycles); + std::cout.flush(); + // Skip the static-dtor cleanup: msquic's RegistrationClose blocks until + // every connection it ever opened is fully drained, which the suite does + // not need to wait on. Everything this test asserts has already happened. + std::_Exit(0); +}