From 74585a0b9874e33752b4395fb01683bebf275d81 Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Wed, 7 Oct 2026 10:54:46 +1100 Subject: [PATCH 1/3] Don't dereference a connection that is already gone when a response arrives --- src/network/transport/quic_transport.cpp | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/network/transport/quic_transport.cpp b/src/network/transport/quic_transport.cpp index 16c73c81..d4bae502 100644 --- a/src/network/transport/quic_transport.cpp +++ b/src/network/transport/quic_transport.cpp @@ -510,13 +510,21 @@ void QuicTransport::_send_on_connection( // Since the request completed it's round-trip if it isn't the "reserverd" stream // (not used for general requests) then we can either add it back to the pool, or // close it if there are more that the active stream price limit + // + // The connection can already be gone: libquic fires a request's callback from its + // destructor when the connection closes, and the connection's streams go with it. + // Then there is nothing to return to the pool or close, and any ids pooled for it + // are dead too. if (stream_id != 0 && _endpoint) { - auto conn = _endpoint->get_conn(conn_id); - - if (conn && conn->get_streams_available() <= ACTIVE_STREAM_PRUNE_LIMIT) - _available_stream_ids[conn_id].insert(stream_id); - else if (auto stream = conn->get_stream(stream_id)) - stream->close(); + if (auto conn = _endpoint->get_conn(conn_id)) { + if (conn->get_streams_available() <= ACTIVE_STREAM_PRUNE_LIMIT) + _available_stream_ids[conn_id].insert(stream_id); + else if ( + auto stream = + conn->get_stream(stream_id)) + stream->close(); + } else + _available_stream_ids.erase(conn_id); } if (resp.timed_out) { From d72c8bca952e0b91f511e16be29353054a31a5a3 Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Wed, 7 Oct 2026 10:55:02 +1100 Subject: [PATCH 2/3] Clear only the closed connection's state when a connection fails --- .../network/transport/quic_transport.hpp | 3 ++- src/network/transport/quic_transport.cpp | 23 +++++++++++++++---- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/include/session/network/transport/quic_transport.hpp b/include/session/network/transport/quic_transport.hpp index d0281f5c..990a3529 100644 --- a/include/session/network/transport/quic_transport.hpp +++ b/include/session/network/transport/quic_transport.hpp @@ -93,7 +93,8 @@ class QuicTransport : public ITransport, public std::enable_shared_from_this error_code, - std::optional custom_error); + std::optional custom_error, + std::optional conn_id = std::nullopt); }; } // namespace session::network \ No newline at end of file diff --git a/src/network/transport/quic_transport.cpp b/src/network/transport/quic_transport.cpp index d4bae502..fa3c8bf5 100644 --- a/src/network/transport/quic_transport.cpp +++ b/src/network/transport/quic_transport.cpp @@ -371,10 +371,14 @@ void QuicTransport::_establish_connection( } }, [weak_self = weak_from_this(), address_pubkey_hex, initiating_req_id]( - oxen::quic::Connection&, uint64_t error_code) { + oxen::quic::Connection& conn, uint64_t error_code) { if (auto self = weak_self.lock()) self->_fail_connection( - address_pubkey_hex, initiating_req_id, error_code, std::nullopt); + address_pubkey_hex, + initiating_req_id, + error_code, + std::nullopt, + conn.reference_id()); }); } catch (const std::exception& e) { _fail_connection(address_pubkey_hex, initiating_req_id, std::nullopt, e.what()); @@ -570,7 +574,8 @@ void QuicTransport::_fail_connection( const std::string& address_pubkey_hex, const std::string& initiating_req_id, std::optional error_code, - std::optional custom_error) { + std::optional custom_error, + std::optional conn_id) { if (error_code == NGTCP2_NO_ERROR) log::info( cat, @@ -613,8 +618,16 @@ void QuicTransport::_fail_connection( address_pubkey_hex, custom_error.value_or("Unknown error")); - // Clear the connection and stream ids - if (auto id = _active_connection_ids.extract(address_pubkey_hex)) + // Clear the connection and stream ids. When the closed connection is known, only its own + // state goes: a newer connection to the same node can be established before an old one's close + // arrives, and clearing by pubkey alone would drop the new connection and orphan the old one's + // stream ids. + if (conn_id) { + _available_stream_ids.erase(*conn_id); + if (auto it = _active_connection_ids.find(address_pubkey_hex); + it != _active_connection_ids.end() && it->second == *conn_id) + _active_connection_ids.erase(it); + } else if (auto id = _active_connection_ids.extract(address_pubkey_hex)) _available_stream_ids.erase(id.mapped()); // Process any waiting verification requests From 0ce0972ecdb1ea11653150bc111828afb70097f6 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Tue, 6 Oct 2026 21:34:50 -0300 Subject: [PATCH 3/3] Say accurately when a response can arrive after its connection is gone On a normal close, libquic fails a stream's pending requests from close_all_streams() while the connection still exists; it only fails them from ~sent_request when a stream outlives its connection. --- src/network/transport/quic_transport.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/network/transport/quic_transport.cpp b/src/network/transport/quic_transport.cpp index fa3c8bf5..d63d3c72 100644 --- a/src/network/transport/quic_transport.cpp +++ b/src/network/transport/quic_transport.cpp @@ -515,10 +515,9 @@ void QuicTransport::_send_on_connection( // (not used for general requests) then we can either add it back to the pool, or // close it if there are more that the active stream price limit // - // The connection can already be gone: libquic fires a request's callback from its - // destructor when the connection closes, and the connection's streams go with it. - // Then there is nothing to return to the pool or close, and any ids pooled for it - // are dead too. + // The connection can be gone by the time this runs, e.g. if the stream outlived it + // (libquic then fails the request from ~sent_request). Then there is nothing to + // return to the pool or close, and any ids pooled for it are dead too. if (stream_id != 0 && _endpoint) { if (auto conn = _endpoint->get_conn(conn_id)) { if (conn->get_streams_available() <= ACTIVE_STREAM_PRUNE_LIMIT)