fix(quic): stop ~ClientQUIC aborting the process on teardown #8

Merged
catbot merged 3 commits from claude/issue-7 into master 2026-08-25 17:10:08 +00:00
3 changed files with 138 additions and 0 deletions
Showing only changes of commit 25e8c84794 - Show all commits

test(quic): cover connection teardown

Every other QUIC test ends in std::_Exit(0), so nothing in the suite ever
ran a ~ClientQUIC. This adds 25 build-up/tear-down cycles with reader
threads parked inside RecieveSync -- the shape a reconnect loop has --
which aborts on ~10/20 runs against the current implementation.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
catbot 2026-08-25 17:06:25 +00:00

View file

@ -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

View file

@ -149,6 +149,7 @@ extern "C" Configuration CrafterBuildProject(std::span<const std::string_view> a
cfg.AddTest("ShouldSendRecieveQUICDatagram").Dependencies({ &cfg });
cfg.AddTest("ShouldSendRecieveQUICStream").Dependencies({ &cfg });
cfg.AddTest("ShouldSurviveAbuseHTTP1").Dependencies({ &cfg });
cfg.AddTest("ShouldSurviveConnectionChurn").Dependencies({ &cfg });
}
return cfg;

View file

@ -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<std::uint16_t>(19200 + i);
const std::string alpn = "churn/1";
QUICServerCredentials serverCreds;
serverCreds.selfSigned = true;
std::mutex serverMutex;
std::vector<QUICStream> serverStreams;
ClientQUIC* accepted = nullptr;
auto listener = std::make_unique<ListenerQUIC>(
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<std::thread> readers;
int started = 0;
client.OnStream([&](QUICStream stream) {
auto shared = std::make_shared<QUICStream>(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);
}