From f7510ca439eb17061d075cc73bef640d2ef104a4 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Wed, 1 Jul 2020 09:39:22 -0700 Subject: [PATCH] quic: additional cleanups on the c++ side PR-URL: https://github.com/nodejs/node/pull/34160 Reviewed-By: Anna Henningsen --- src/quic/node_quic_http3_application.cc | 4 +- src/quic/node_quic_session.cc | 74 +++++++++++++------------ src/quic/node_quic_session.h | 51 ++++++----------- 3 files changed, 58 insertions(+), 71 deletions(-) diff --git a/src/quic/node_quic_http3_application.cc b/src/quic/node_quic_http3_application.cc index 7dc41a392e54df..150734bb81c925 100644 --- a/src/quic/node_quic_http3_application.cc +++ b/src/quic/node_quic_http3_application.cc @@ -594,8 +594,8 @@ void Http3Application::StreamClosed( int64_t stream_id, uint64_t app_error_code) { BaseObjectPtr stream = session()->FindStream(stream_id); - CHECK(stream); - stream->ReceiveData(1, nullptr, 0, 0); + if (stream) + stream->ReceiveData(1, nullptr, 0, 0); session()->listener()->OnStreamClose(stream_id, app_error_code); } diff --git a/src/quic/node_quic_session.cc b/src/quic/node_quic_session.cc index d28f97cdba8fb8..5f102eb2f9362f 100644 --- a/src/quic/node_quic_session.cc +++ b/src/quic/node_quic_session.cc @@ -757,6 +757,10 @@ void QuicSession::RandomConnectionIDStrategy( # define HAVE_SSL_TRACE 1 #endif +QuicCryptoContext::~QuicCryptoContext() { + // Free any remaining crypto handshake data (if any) + Cancel(); +} void QuicCryptoContext::MemoryInfo(MemoryTracker* tracker) const { tracker->TrackField("initial_crypto", handshake_[0]); @@ -1470,8 +1474,6 @@ QuicSession::QuicSession( QuicSession::~QuicSession() { CHECK(!Ngtcp2CallbackScope::InNgtcp2CallbackScope(this)); - crypto_context_->Cancel(); - connection_.reset(); QuicSessionListener* listener_ = listener(); listener_->OnSessionDestroyed(); @@ -1637,7 +1639,9 @@ void QuicSession::AddStream(BaseObjectPtr stream) { // not immediately torn down, but is allowed to drain // properly per the QUIC spec description of "immediate close". void QuicSession::ImmediateClose() { - if (is_closing() || is_silent_closing()) + // If ImmediateClose or SilentClose has already been called, + // do not re-enter. + if (is_closing()) return; set_closing(); @@ -1649,6 +1653,35 @@ void QuicSession::ImmediateClose() { listener()->OnSessionClose(err); } +// Silent Close must start with the JavaScript side, which must +// clean up state, abort any still existing QuicSessions, then +// destroy the handle when done. The most important characteristic +// of the SilentClose is that no frames are sent to the peer. +// +// When a valid stateless reset is received, the connection is +// immediately and unrecoverably closed at the ngtcp2 level. +// Specifically, it will be put into the draining_period so +// absolutely no frames can be sent. What we need to do is +// notify the JavaScript side and destroy the connection with +// a flag set that indicates stateless reset. +void QuicSession::SilentClose() { + CHECK(!is_silent_closing()); + set_silent_closing(); + set_closing(); + + QuicError err = last_error(); + Debug(this, + "Silent close with %s code %" PRIu64 " (stateless reset? %s)", + err.family_name(), + err.code, + is_stateless_reset() ? "yes" : "no"); + + int flags = QuicSessionListener::SESSION_CLOSE_FLAG_SILENT; + if (is_stateless_reset()) + flags |= QuicSessionListener::SESSION_CLOSE_FLAG_STATELESS_RESET; + listener()->OnSessionClose(err, flags); +} + // Creates a new stream object and passes it off to the javascript side. // This has to be called from within a handlescope/contextscope. BaseObjectPtr QuicSession::CreateStream(int64_t stream_id) { @@ -1958,7 +1991,7 @@ bool QuicSession::ReceivePacket( // then immediately close the connection. if (err == NGTCP2_ERR_RETRY && is_server()) { socket()->SendRetry(scid_, dcid_, local_address_, remote_address_); - ImmediateClose(); + SilentClose(); break; } set_last_error(QUIC_ERROR_SESSION, err); @@ -2050,7 +2083,7 @@ void QuicSession::RemoveFromSocket() { void QuicSession::RemoveStream(int64_t stream_id) { Debug(this, "Removing stream %" PRId64, stream_id); - // ngtcp2 does no extend the max streams count automatically + // ngtcp2 does not extend the max streams count automatically // except in very specific conditions, none of which apply // once we've gotten this far. We need to manually extend when // a remote peer initiated stream is removed. @@ -2104,6 +2137,8 @@ bool QuicSession::SendConnectionClose() { // it multiple times; whereas for clients, we will serialize it // once and send once only. QuicError error = last_error(); + Debug(this, "Connection Close code: %" PRIu64 " (family: %s)", + error.code, error.family_name()); // If initial keys have not yet been installed, use the alternative // ImmediateConnectionClose to send a stateless connection close to @@ -2322,34 +2357,6 @@ void QuicSession::ResumeStream(int64_t stream_id) { application()->ResumeStream(stream_id); } -// Silent Close must start with the JavaScript side, which must -// clean up state, abort any still existing QuicSessions, then -// destroy the handle when done. The most important characteristic -// of the SilentClose is that no frames are sent to the peer. -// -// When a valid stateless reset is received, the connection is -// immediately and unrecoverably closed at the ngtcp2 level. -// Specifically, it will be put into the draining_period so -// absolutely no frames can be sent. What we need to do is -// notify the JavaScript side and destroy the connection with -// a flag set that indicates stateless reset. -void QuicSession::SilentClose() { - CHECK(!is_silent_closing()); - set_silent_closing(); - set_closing(); - - QuicError err = last_error(); - Debug(this, - "Silent close with %s code %" PRIu64 " (stateless reset? %s)", - err.family_name(), - err.code, - is_stateless_reset() ? "yes" : "no"); - - int flags = QuicSessionListener::SESSION_CLOSE_FLAG_SILENT; - if (is_stateless_reset()) - flags |= QuicSessionListener::SESSION_CLOSE_FLAG_STATELESS_RESET; - listener()->OnSessionClose(err, flags); -} // Begin connection close by serializing the CONNECTION_CLOSE packet. // There are two variants: one to serialize an application close, the // other to serialize a protocol close. The frames are generally @@ -2508,7 +2515,6 @@ void QuicSession::UpdateIdleTimer() { // serialize stream data that is being sent initially. bool QuicSession::WritePackets(const char* diagnostic_label) { CHECK(!Ngtcp2CallbackScope::InNgtcp2CallbackScope(this)); - CHECK(!is_destroyed()); // During the draining period, we must not send any frames at all. if (is_in_draining_period()) diff --git a/src/quic/node_quic_session.h b/src/quic/node_quic_session.h index 8c22fe3bdab534..3087ecf12c947b 100644 --- a/src/quic/node_quic_session.h +++ b/src/quic/node_quic_session.h @@ -369,6 +369,14 @@ class JSQuicSessionListener : public QuicSessionListener { // handshake details on behalf of a QuicSession. class QuicCryptoContext : public MemoryRetainer { public: + inline QuicCryptoContext( + QuicSession* session, + BaseObjectPtr secure_context, + ngtcp2_crypto_side side, + uint32_t options); + + ~QuicCryptoContext() override; + inline uint64_t Cancel(); // Outgoing crypto data must be retained in memory until it is @@ -482,12 +490,6 @@ class QuicCryptoContext : public MemoryRetainer { SET_SELF_SIZE(QuicCryptoContext) private: - inline QuicCryptoContext( - QuicSession* session, - BaseObjectPtr secure_context, - ngtcp2_crypto_side side, - uint32_t options); - bool SetSecrets( ngtcp2_crypto_level level, const uint8_t* rx_secret, @@ -1038,37 +1040,16 @@ class QuicSession : public AsyncWrap, inline void DecreaseAllocatedSize(size_t size); - // Immediately close the QuicSession. All currently open - // streams are implicitly reset and closed with RESET_STREAM - // and STOP_SENDING frames transmitted as necessary. A - // CONNECTION_CLOSE frame will be sent and the session - // will enter the closing period until either the idle - // timeout period elapses or until the QuicSession is - // explicitly destroyed. During the closing period, - // the only frames that may be transmitted to the peer - // are repeats of the already sent CONNECTION_CLOSE. - // - // The CONNECTION_CLOSE will use the error code set using - // the most recent call to set_last_error() + // Triggers an "immediate close" on the QuicSession. + // This will round trip through JavaScript, causing + // all currently open streams to be closed and ultimately + // send a CONNECTION_CLOSE to the connected peer before + // terminating the connection. void ImmediateClose(); - // Silently, and immediately close the QuicSession. This is - // generally only done during an idle timeout. That is, per - // the QUIC specification, if the session remains idle for - // longer than both the advertised idle timeout and three - // times the current probe timeout (PTO). In such cases, all - // currently open streams are implicitly reset and closed - // without sending corresponding RESET_STREAM and - // STOP_SENDING frames, the connection state is - // discarded, and the QuicSession is destroyed without - // sending a CONNECTION_CLOSE frame. - // - // Silent close may also be used to explicitly destroy - // a QuicSession that has either already entered the - // closing or draining periods; or in response to user - // code requests to forcefully terminate a QuicSession - // without transmitting any additional frames to the - // peer. + // Silently and immediately close the QuicSession. This is + // typically only done during an idle timeout or when sending + // a retry packet. void SilentClose(); void PushListener(QuicSessionListener* listener);