ClientQUIC: a send racing teardown leaks a stream handle — hangs MsQuicRegistrationClose at exit, and intermittently aborts a worker thread #9

New issue

Closed

opened 2026-08-25 22:42:23 +00:00 by jorijnvdgraaf · 0 comments

jorijnvdgraaf commented

2026-08-25 22:42:23 +00:00

Owner

Summary

Follow-up to #7 / #8. That fix is good — #7's exact reproducer now passes 0/25
on master (9a5cab8), and its MsQuicConnectionClose ← ~ClientQUIC backtrace
is gone. But a related fault survives, and it shows up when a client sends on a
stream while the connection is being torn down — the shape any long-lived
client with periodic acks or keepalives has.

Two symptoms, which I believe are one cause:

Hang at exit (~10 of 15 runs). The churn loop finishes, then the process
blocks forever in MsQuicRegistrationClose.
Intermittent abort (~1 in 25 runs here; ~1 in 6–18 in the real client this
came from). quic_bugcheck on an msquic worker thread, not on the thread
running the destructor.

Impact

Any client that reconnects and sends periodically. The reconnect loop itself is
fine now; it is the concurrent send that reintroduces the problem. In the ingress
client this came from, an ack timer sends every 50 ms for the life of a
connection, so every reconnect races a send against teardown.

The hang is arguably the worse of the two: the work completes, the process just
never exits, so a supervisor sees a live-but-wedged process rather than a crash
loop.

What is different from #7

#7's reproducer opens server-initiated streams and parks readers in
RecieveSync. That is fixed. This one adds one thing to it: a client-opened
bidirectional stream with a thread sending on it across the teardown.

The server must drain that stream, or SendSync blocks on flow control and the
hang is the reproducer's own fault rather than a bug — the version below drains
it, and still fails.

Reproducer

tests/ShouldSurviveConnectionChurn/main.cpp, registered with
cfg.AddTest("ShouldSurviveConnectionChurn").Dependencies({ &cfg });

import Crafter.Network;
import Crafter.Thread;
import std;
using namespace Crafter;

namespace {
    constexpr int cycles = 25;
    constexpr int streamsPerConnection = 3;
}

int main() {
    ThreadPool::Start();

    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) {
                peer->OnStream([&](QUICStream inbound) {
                    auto shared = std::make_shared<QUICStream>(std::move(inbound));
                    std::thread([shared] {
                        try { while (true) (void)shared->RecieveSync(); } catch (...) {}
                    }).detach();
                });
                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);

            // Each inbound stream gets a thread that stays blocked in
            // RecieveSync for the life of the connection -- the shape a
            // long-lived streaming client has. Teardown therefore happens
            // while readers are parked inside the receive path.
            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;
                });
            }

            // A client-opened bidirectional control stream with a timer
            // thread sending on it, which is the shape a long-lived client
            // with periodic acks/keepalives has. The sends keep going while
            // the connection is torn down underneath them.
            QUICStream control = client.OpenStream(/*unidirectional=*/false);
            std::atomic<bool> sending{ true };
            std::thread sender([&] {
                while (sending) {
                    try {
                        const char tick[] = "ack";
                        control.SendSync(tick, sizeof(tick) - 1, /*finish=*/false);
                    } catch (...) {
                        return;
                    }
                    std::this_thread::sleep_for(std::chrono::milliseconds(2));
                }
            });

            client.Stop();
            sending = false;
            if (sender.joinable()) sender.join();
            for (std::thread& reader : readers) {
                if (reader.joinable()) reader.join();
            }
            // `client` is destroyed here, once the readers have unwound.
        }

        {
            std::lock_guard lock(serverMutex);
            serverStreams.clear();
            if (accepted) accepted->Stop();
        }
        listener->Stop();
        listener.reset();
    }

    std::println("survived {} connection teardowns", cycles);
    return 0;
}

$ for i in $(seq 1 15); do timeout 20 ./ShouldSurviveConnectionChurn >/dev/null 2>&1; \
    case $? in 0) ok=$((ok+1));; 124) hang=$((hang+1));; 134) abort=$((abort+1));; esac; done
ok=5  hang=10  abort=0   (of 15)

Over a longer run the abort appears too (1 of 25). Neither symptom reproduces
under gdb — 15/15 clean — so coredumpctl, or kill -ABRT on the hung
process, is the practical way to get a stack.

Stacks

Hang — kill -ABRT on a process stuck ~12 s after the loop finished:

#2  pthread_cond_wait ()             libc.so.6
#3  CxPlatRundownReleaseAndWait ()   libmsquic.so.2
#4  QuicRegistrationClose ()         libmsquic.so.2
#5  MsQuicRegistrationClose ()       libmsquic.so.2
#6  <ShouldSurviveConnectionChurn>
#7  exit ()                          libc.so.6
#8  __libc_start_main ()             libc.so.6

Abort — from the downstream client, same reproducer shape:

#3  quic_bugcheck ()               libmsquic.so.2
#4  QuicOperationDequeue ()        libmsquic.so.2
#5  QuicConnDrainOperations ()     libmsquic.so.2
#6  QuicWorkerProcessConnection () libmsquic.so.2
#7  QuicWorkerLoop ()              libmsquic.so.2
#8  CxPlatWorkerThread ()          libmsquic.so.2

while the app thread sits in std::thread::join.

Diagnosis

The hang stack is the solid part. CxPlatRundownReleaseAndWait inside
MsQuicRegistrationClose means the registration's rundown never reaches zero:
an msquic handle was never closed. QUICStream::Stop only initiates a
shutdown, and the actual StreamClose happens in FinalizeClose off the
SHUTDOWN_COMPLETE callback — so a stream whose SHUTDOWN_COMPLETE never
arrives never closes its handle, and the registration can never run down.

#8 added exactly the right guard for this — ~ClientQUIC now drains a shared
StreamRegistry before closing the connection — but the drain is bounded at
5 s and then proceeds anyway:

The count lives in a shared StreamRegistry rather than behind the
ClientQUIC*, so a stream finalising after the wait gave up decrements a
live object.

That comment is about not corrupting memory when the wait gives up, which it
achieves. It does not address what happens to the stream that never finalised:
its handle stays open. That would explain both symptoms from one cause —

the connection is closed with streams msquic still considers open, which is
what the worker thread trips over while draining that connection's operations
(the abort); and
the stream handle is never closed, so the registration rundown at exit()
blocks forever (the hang).

The part I am less sure of is why SHUTDOWN_COMPLETE goes missing when a send
is in flight, and whether the right fix is to make the drain unbounded, to force
StreamClose on the streams still outstanding when it expires, or to stop
accepting sends once teardown has begun. I have not tried to fix it — flagging
it because it is shared infrastructure and I did not want to guess at the right
change.

Notes

Found while building a long-lived QUIC ingress client (the same one behind #7).
It is not blocking there — that client is gaining durable cursors, so an abort is
a restart rather than data loss — but it wants a deliberate fix rather than being
worked around downstream.

## Summary Follow-up to #7 / #8. That fix is good — #7's exact reproducer now passes **0/25** on `master` (9a5cab8), and its `MsQuicConnectionClose` ← `~ClientQUIC` backtrace is gone. But a related fault survives, and it shows up when a client **sends on a stream while the connection is being torn down** — the shape any long-lived client with periodic acks or keepalives has. Two symptoms, which I believe are one cause: - **Hang at exit** (~10 of 15 runs). The churn loop finishes, then the process blocks forever in `MsQuicRegistrationClose`. - **Intermittent abort** (~1 in 25 runs here; ~1 in 6–18 in the real client this came from). `quic_bugcheck` on an msquic *worker* thread, not on the thread running the destructor. ## Impact Any client that reconnects and sends periodically. The reconnect loop itself is fine now; it is the concurrent send that reintroduces the problem. In the ingress client this came from, an ack timer sends every 50 ms for the life of a connection, so every reconnect races a send against teardown. The hang is arguably the worse of the two: the work completes, the process just never exits, so a supervisor sees a live-but-wedged process rather than a crash loop. ## What is different from #7 #7's reproducer opens server-initiated streams and parks readers in `RecieveSync`. That is fixed. This one adds one thing to it: a client-opened **bidirectional** stream with a thread sending on it across the teardown. The server must drain that stream, or `SendSync` blocks on flow control and the hang is the reproducer's own fault rather than a bug — the version below drains it, and still fails. ## Reproducer `tests/ShouldSurviveConnectionChurn/main.cpp`, registered with `cfg.AddTest("ShouldSurviveConnectionChurn").Dependencies({ &cfg });` ```cpp import Crafter.Network; import Crafter.Thread; import std; using namespace Crafter; namespace { constexpr int cycles = 25; constexpr int streamsPerConnection = 3; } int main() { ThreadPool::Start(); 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) { peer->OnStream([&](QUICStream inbound) { auto shared = std::make_shared<QUICStream>(std::move(inbound)); std::thread([shared] { try { while (true) (void)shared->RecieveSync(); } catch (...) {} }).detach(); }); 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); // Each inbound stream gets a thread that stays blocked in // RecieveSync for the life of the connection -- the shape a // long-lived streaming client has. Teardown therefore happens // while readers are parked inside the receive path. 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; }); } // A client-opened bidirectional control stream with a timer // thread sending on it, which is the shape a long-lived client // with periodic acks/keepalives has. The sends keep going while // the connection is torn down underneath them. QUICStream control = client.OpenStream(/*unidirectional=*/false); std::atomic<bool> sending{ true }; std::thread sender([&] { while (sending) { try { const char tick[] = "ack"; control.SendSync(tick, sizeof(tick) - 1, /*finish=*/false); } catch (...) { return; } std::this_thread::sleep_for(std::chrono::milliseconds(2)); } }); client.Stop(); sending = false; if (sender.joinable()) sender.join(); for (std::thread& reader : readers) { if (reader.joinable()) reader.join(); } // `client` is destroyed here, once the readers have unwound. } { std::lock_guard lock(serverMutex); serverStreams.clear(); if (accepted) accepted->Stop(); } listener->Stop(); listener.reset(); } std::println("survived {} connection teardowns", cycles); return 0; } ``` ``` $ for i in $(seq 1 15); do timeout 20 ./ShouldSurviveConnectionChurn >/dev/null 2>&1; \ case $? in 0) ok=$((ok+1));; 124) hang=$((hang+1));; 134) abort=$((abort+1));; esac; done ok=5 hang=10 abort=0 (of 15) ``` Over a longer run the abort appears too (1 of 25). Neither symptom reproduces under `gdb` — 15/15 clean — so `coredumpctl`, or `kill -ABRT` on the hung process, is the practical way to get a stack. ## Stacks **Hang** — `kill -ABRT` on a process stuck ~12 s after the loop finished: ``` #2 pthread_cond_wait () libc.so.6 #3 CxPlatRundownReleaseAndWait () libmsquic.so.2 #4 QuicRegistrationClose () libmsquic.so.2 #5 MsQuicRegistrationClose () libmsquic.so.2 #6 <ShouldSurviveConnectionChurn> #7 exit () libc.so.6 #8 __libc_start_main () libc.so.6 ``` **Abort** — from the downstream client, same reproducer shape: ``` #3 quic_bugcheck () libmsquic.so.2 #4 QuicOperationDequeue () libmsquic.so.2 #5 QuicConnDrainOperations () libmsquic.so.2 #6 QuicWorkerProcessConnection () libmsquic.so.2 #7 QuicWorkerLoop () libmsquic.so.2 #8 CxPlatWorkerThread () libmsquic.so.2 ``` while the app thread sits in `std::thread::join`. ## Diagnosis The hang stack is the solid part. `CxPlatRundownReleaseAndWait` inside `MsQuicRegistrationClose` means the registration's rundown never reaches zero: **an msquic handle was never closed**. `QUICStream::Stop` only initiates a shutdown, and the actual `StreamClose` happens in `FinalizeClose` off the `SHUTDOWN_COMPLETE` callback — so a stream whose `SHUTDOWN_COMPLETE` never arrives never closes its handle, and the registration can never run down. `#8` added exactly the right guard for this — `~ClientQUIC` now drains a shared `StreamRegistry` before closing the connection — but the drain is **bounded at 5 s and then proceeds anyway**: > The count lives in a shared StreamRegistry rather than behind the > ClientQUIC*, so a stream finalising after the wait gave up decrements a > live object. That comment is about not corrupting memory when the wait gives up, which it achieves. It does not address what happens to the stream that never finalised: its handle stays open. That would explain both symptoms from one cause — - the connection is closed with streams msquic still considers open, which is what the worker thread trips over while draining that connection's operations (the abort); and - the stream handle is never closed, so the registration rundown at `exit()` blocks forever (the hang). The part I am less sure of is *why* `SHUTDOWN_COMPLETE` goes missing when a send is in flight, and whether the right fix is to make the drain unbounded, to force `StreamClose` on the streams still outstanding when it expires, or to stop accepting sends once teardown has begun. I have not tried to fix it — flagging it because it is shared infrastructure and I did not want to guess at the right change. ## Notes Found while building a long-lived QUIC ingress client (the same one behind #7). It is not blocking there — that client is gaining durable cursors, so an abort is a restart rather than data loss — but it wants a deliberate fix rather than being worked around downstream.

jorijnvdgraaf added the

claude:ready

label

2026-08-25 22:45:11 +00:00