From 76f941b72c8fc81dd69662774115915930632a53 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Tue, 30 Jun 2020 19:42:52 -0700 Subject: [PATCH 01/11] quic: continued refactoring for quic_stream/quic_session --- src/quic/node_quic_default_application.cc | 5 +++- src/quic/node_quic_http3_application.cc | 5 +++- src/quic/node_quic_session.cc | 5 ---- src/quic/node_quic_session.h | 31 ----------------------- src/quic/node_quic_stream-inl.h | 28 +++++++++++++++----- src/quic/node_quic_stream.cc | 31 ++++++++++++++++++++--- src/quic/node_quic_stream.h | 6 +++-- test/parallel/test-quic-statelessreset.js | 3 ++- 8 files changed, 63 insertions(+), 51 deletions(-) diff --git a/src/quic/node_quic_default_application.cc b/src/quic/node_quic_default_application.cc index ea84099d70ae61..4c253b3787727b 100644 --- a/src/quic/node_quic_default_application.cc +++ b/src/quic/node_quic_default_application.cc @@ -93,7 +93,10 @@ bool DefaultApplication::ReceiveStreamData( if (!stream) { // Shutdown the stream explicitly if the session is being closed. if (session()->is_gracefully_closing()) { - session()->ResetStream(stream_id, NGTCP2_ERR_CLOSING); + ngtcp2_conn_shutdown_stream( + session()->connection(), + stream_id, + NGTCP2_ERR_CLOSING); return true; } diff --git a/src/quic/node_quic_http3_application.cc b/src/quic/node_quic_http3_application.cc index 673e8a4eace9ea..44f874cf7aeb8f 100644 --- a/src/quic/node_quic_http3_application.cc +++ b/src/quic/node_quic_http3_application.cc @@ -702,7 +702,10 @@ void Http3Application::PushStream( void Http3Application::SendStopSending( int64_t stream_id, uint64_t app_error_code) { - session()->ResetStream(stream_id, app_error_code); + ngtcp2_conn_shutdown_stream_read( + session()->connection(), + stream_id, + app_error_code); } void Http3Application::EndStream(int64_t stream_id) { diff --git a/src/quic/node_quic_session.cc b/src/quic/node_quic_session.cc index d058035a0b7896..d513b163cda7f1 100644 --- a/src/quic/node_quic_session.cc +++ b/src/quic/node_quic_session.cc @@ -2328,11 +2328,6 @@ void QuicSession::ResumeStream(int64_t stream_id) { application()->ResumeStream(stream_id); } -void QuicSession::ResetStream(int64_t stream_id, uint64_t code) { - SendSessionScope scope(this); - CHECK_EQ(ngtcp2_conn_shutdown_stream(connection(), stream_id, code), 0); -} - // 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 diff --git a/src/quic/node_quic_session.h b/src/quic/node_quic_session.h index 78fb7ee0a30540..1e9341beecfe05 100644 --- a/src/quic/node_quic_session.h +++ b/src/quic/node_quic_session.h @@ -957,37 +957,6 @@ class QuicSession : public AsyncWrap, const StreamsMap& streams() const { return streams_; } - // ResetStream will cause ngtcp2 to queue a - // RESET_STREAM and STOP_SENDING frame, as appropriate, - // for the given stream_id. For a locally-initiated - // unidirectional stream, only a RESET_STREAM frame - // will be scheduled and the stream will be immediately - // closed. For a bi-directional stream, a STOP_SENDING - // frame will be sent. - // - // It is important to note that the QuicStream is - // not destroyed immediately following ShutdownStream. - // The sending QuicSession will not close the stream - // until the RESET_STREAM is acknowledged. - // - // Once the RESET_STREAM is sent, the QuicSession - // should not send any new frames for the stream, - // and all inbound stream frames should be discarded. - // Once ngtcp2 receives the appropriate notification - // that the RESET_STREAM has been acknowledged, the - // stream will be closed. - // - // Once the stream has been closed, it will be - // destroyed and memory will be freed. User code - // can request that a stream be immediately and - // abruptly destroyed without calling ShutdownStream. - // Likewise, an idle timeout may cause the stream - // to be silently destroyed without calling - // ShutdownStream. - void ResetStream( - int64_t stream_id, - uint64_t error_code = NGTCP2_APP_NOERROR); - void ResumeStream(int64_t stream_id); // Submits informational headers to the QUIC Application diff --git a/src/quic/node_quic_stream-inl.h b/src/quic/node_quic_stream-inl.h index 3dd1fc216d9783..546e867ee2db7e 100644 --- a/src/quic/node_quic_stream-inl.h +++ b/src/quic/node_quic_stream-inl.h @@ -132,19 +132,33 @@ void QuicStream::Commit(size_t amount) { streambuf_.Seek(amount); } +// ResetStream will cause ngtcp2 to queue a RESET_STREAM and STOP_SENDING +// frame, as appropriate, for the given stream_id. For a locally-initiated +// unidirectional stream, only a RESET_STREAM frame will be scheduled and +// the stream will be immediately closed. For a bidirectional stream, a +// STOP_SENDING frame will be sent. void QuicStream::ResetStream(uint64_t app_error_code) { - // On calling shutdown, the stream will no longer be - // readable or writable, all any pending data in the - // streambuf_ will be canceled, and all data pending - // to be acknowledged at the ngtcp2 level will be - // abandoned. - BaseObjectPtr ptr(session_); + QuicSession::SendSessionScope send_scope(session()); + ngtcp2_conn_shutdown_stream( + session()->connection(), + stream_id_, + app_error_code); set_flag(QUICSTREAM_FLAG_READ_CLOSED); - session_->ResetStream(stream_id_, app_error_code); streambuf_.Cancel(); streambuf_.End(); } +// StopSending will cause ngtcp2 to queue a STOP_SENDING frame if the +// stream is still inbound readable. +void QuicStream::StopSending(uint64_t app_error_code) { + QuicSession::SendSessionScope send_scope(session()); + ngtcp2_conn_shutdown_stream_read( + session()->connection(), + stream_id_, + app_error_code); + set_flag(QUICSTREAM_FLAG_READ_CLOSED); +} + void QuicStream::Schedule(Queue* queue) { if (!stream_queue_.IsEmpty()) // Already scheduled? return; diff --git a/src/quic/node_quic_stream.cc b/src/quic/node_quic_stream.cc index ce8cc78a1ec8c5..ba447e760d206a 100644 --- a/src/quic/node_quic_stream.cc +++ b/src/quic/node_quic_stream.cc @@ -118,21 +118,31 @@ std::string QuicStream::diagnostic_name() const { ", " + session_->diagnostic_name() + ")"; } -void QuicStream::Destroy() { +void QuicStream::Destroy(QuicError* error) { if (is_destroyed()) return; + + QuicSession::SendSessionScope send_scope(session()); + set_flag(QUICSTREAM_FLAG_DESTROYED); set_flag(QUICSTREAM_FLAG_READ_CLOSED); - streambuf_.End(); + + // In case this stream is scheduled for sending, remove it + // from the schedule queue + Unschedule(); // If there is data currently buffered in the streambuf_, // then cancel will call out to invoke an arbitrary // JavaScript callback (the on write callback). Within // that callback, however, the QuicStream will no longer // be usable to send or receive data. + streambuf_.End(); streambuf_.Cancel(); CHECK_EQ(streambuf_.length(), 0); + // Attempt to send a shutdown signal to the remote peer + ResetStream(error != nullptr ? error->code : NGTCP2_NO_ERROR); + // The QuicSession maintains a map of std::unique_ptrs to // QuicStream instances. Removing this here will cause // this QuicStream object to be deconstructed, so the @@ -411,9 +421,11 @@ void OpenBidirectionalStream(const FunctionCallbackInfo& args) { } void QuicStreamDestroy(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); QuicStream* stream; ASSIGN_OR_RETURN_UNWRAP(&stream, args.Holder()); - stream->Destroy(); + QuicError error(env, args[0], args[1], QUIC_ERROR_APPLICATION); + stream->Destroy(&error); } void QuicStreamReset(const FunctionCallbackInfo& args) { @@ -428,6 +440,18 @@ void QuicStreamReset(const FunctionCallbackInfo& args) { error.code : static_cast(NGTCP2_NO_ERROR)); } +void QuicStreamStopSending(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + QuicStream* stream; + ASSIGN_OR_RETURN_UNWRAP(&stream, args.Holder()); + + QuicError error(env, args[0], args[1], QUIC_ERROR_APPLICATION); + + stream->StopSending( + error.family == QUIC_ERROR_APPLICATION ? + error.code : static_cast(NGTCP2_NO_ERROR)); +} + // Requests transmission of a block of informational headers. Not all // QUIC Applications will support headers. If headers are not supported, // This will set the return value to false, otherwise the return value @@ -494,6 +518,7 @@ void QuicStream::Initialize( streamt->Set(env->owner_symbol(), Null(env->isolate())); env->SetProtoMethod(stream, "destroy", QuicStreamDestroy); env->SetProtoMethod(stream, "resetStream", QuicStreamReset); + env->SetProtoMethod(stream, "stopSending", QuicStreamStopSending); env->SetProtoMethod(stream, "id", QuicStreamGetID); env->SetProtoMethod(stream, "submitInformation", QuicStreamSubmitInformation); env->SetProtoMethod(stream, "submitHeaders", QuicStreamSubmitHeaders); diff --git a/src/quic/node_quic_stream.h b/src/quic/node_quic_stream.h index d8297f300ba85c..0d0809c74df0ed 100644 --- a/src/quic/node_quic_stream.h +++ b/src/quic/node_quic_stream.h @@ -275,7 +275,7 @@ class QuicStream : public AsyncWrap, void Acknowledge(uint64_t offset, size_t datalen); // Destroy the QuicStream and render it no longer usable. - void Destroy(); + void Destroy(QuicError* error = nullptr); // Buffers chunks of data to be written to the QUIC connection. int DoWrite( @@ -312,7 +312,9 @@ class QuicStream : public AsyncWrap, // Resets the QUIC stream, sending a signal to the peer that // no additional data will be transmitted for this stream. - inline void ResetStream(uint64_t app_error_code = 0); + inline void ResetStream(uint64_t app_error_code = NGTCP2_NO_ERROR); + + inline void StopSending(uint64_t app_error_code = NGTCP2_NO_ERROR); // Submits informational headers. Returns false if headers are not // supported on the underlying QuicApplication. diff --git a/test/parallel/test-quic-statelessreset.js b/test/parallel/test-quic-statelessreset.js index b8d0279660c3a5..370fbf15de9acf 100644 --- a/test/parallel/test-quic-statelessreset.js +++ b/test/parallel/test-quic-statelessreset.js @@ -44,7 +44,8 @@ server.on('session', common.mustCall((session) => { server.on('close', common.mustCall(() => { // Verify stats recording - assert.strictEqual(server.statelessResetCount, 1n); + console.log(server.statelessResetCount); + assert(server.statelessResetCount >= 1n); })); server.on('ready', common.mustCall(() => { From 8a702bd435cdff2ea778cf7d184da8c384ace2c9 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Wed, 1 Jul 2020 07:29:19 -0700 Subject: [PATCH 02/11] quic: refactor native object flags for better readability Use is_* and set_* pattern for native object flags to improve readability in the code. --- lib/internal/quic/core.js | 8 +- src/quic/node_quic_default_application.cc | 2 +- src/quic/node_quic_http3_application.cc | 2 +- src/quic/node_quic_session-inl.h | 43 +++---- src/quic/node_quic_session.cc | 110 +++++++++--------- src/quic/node_quic_session.h | 134 ++++++++++------------ src/quic/node_quic_socket-inl.h | 20 ++-- src/quic/node_quic_socket.cc | 15 ++- src/quic/node_quic_socket.h | 87 +++++++------- src/quic/node_quic_stream-inl.h | 33 +----- src/quic/node_quic_stream.cc | 12 +- src/quic/node_quic_stream.h | 67 +++++------ 12 files changed, 234 insertions(+), 299 deletions(-) diff --git a/lib/internal/quic/core.js b/lib/internal/quic/core.js index 446bec3761374b..90727194246450 100644 --- a/lib/internal/quic/core.js +++ b/lib/internal/quic/core.js @@ -966,9 +966,11 @@ class QuicSocket extends EventEmitter { state.lookup = lookup || (type === AF_INET6 ? lookup6 : lookup4); state.server = server; - const socketOptions = - (validateAddress ? QUICSOCKET_OPTIONS_VALIDATE_ADDRESS : 0) | - (validateAddressLRU ? QUICSOCKET_OPTIONS_VALIDATE_ADDRESS_LRU : 0); + let socketOptions = 0; + if (validateAddress) + socketOptions |= (1 << QUICSOCKET_OPTIONS_VALIDATE_ADDRESS); + if (validateAddressLRU) + socketOptions |= (1 << QUICSOCKET_OPTIONS_VALIDATE_ADDRESS_LRU); this[kSetHandle]( new QuicSocketHandle( diff --git a/src/quic/node_quic_default_application.cc b/src/quic/node_quic_default_application.cc index 4c253b3787727b..5af8dc098d6027 100644 --- a/src/quic/node_quic_default_application.cc +++ b/src/quic/node_quic_default_application.cc @@ -92,7 +92,7 @@ bool DefaultApplication::ReceiveStreamData( BaseObjectPtr stream = session()->FindStream(stream_id); if (!stream) { // Shutdown the stream explicitly if the session is being closed. - if (session()->is_gracefully_closing()) { + if (session()->is_graceful_closing()) { ngtcp2_conn_shutdown_stream( session()->connection(), stream_id, diff --git a/src/quic/node_quic_http3_application.cc b/src/quic/node_quic_http3_application.cc index 44f874cf7aeb8f..7dc41a392e54df 100644 --- a/src/quic/node_quic_http3_application.cc +++ b/src/quic/node_quic_http3_application.cc @@ -603,7 +603,7 @@ BaseObjectPtr Http3Application::FindOrCreateStream( int64_t stream_id) { BaseObjectPtr stream = session()->FindStream(stream_id); if (!stream) { - if (session()->is_gracefully_closing()) { + if (session()->is_graceful_closing()) { nghttp3_conn_close_stream(connection(), stream_id, NGTCP2_ERR_CLOSING); return {}; } diff --git a/src/quic/node_quic_session-inl.h b/src/quic/node_quic_session-inl.h index 5acb8a2e9350ef..339df8210f4a09 100644 --- a/src/quic/node_quic_session-inl.h +++ b/src/quic/node_quic_session-inl.h @@ -112,7 +112,10 @@ void QuicCryptoContext::Keylog(const char* line) { void QuicCryptoContext::OnClientHelloDone() { // Continue the TLS handshake when this function exits // otherwise it will stall and fail. - TLSHandshakeScope handshake(this, &in_client_hello_); + TLSHandshakeScope handshake_scope( + this, + [&]() { set_in_client_hello(false); }); + // Disable the callback at this point so we don't loop continuously session_->state_[IDX_QUIC_SESSION_STATE_CLIENT_HELLO_ENABLED] = 0; } @@ -129,8 +132,8 @@ void QuicCryptoContext::ResumeHandshake() { // For 0RTT, this sets the TLS session data from the given buffer. bool QuicCryptoContext::set_session(crypto::SSLSessionPointer session) { if (side_ == NGTCP2_CRYPTO_SIDE_CLIENT && session != nullptr) { - early_data_ = - SSL_SESSION_get_max_early_data(session.get()) == 0xffffffffUL; + set_early_data( + SSL_SESSION_get_max_early_data(session.get()) == 0xffffffffUL); } return crypto::SetTLSSession(ssl_, std::move(session)); } @@ -186,7 +189,7 @@ std::string QuicCryptoContext::selected_alpn() const { bool QuicCryptoContext::early_data() const { return - (early_data_ && + (is_early_data() && SSL_get_early_data_status(ssl_.get()) == SSL_EARLY_DATA_ACCEPTED) || SSL_get_max_early_data(ssl_.get()) == 0xffffffffUL; } @@ -300,13 +303,13 @@ void QuicSession::ExtendOffset(size_t amount) { // Copies the local transport params into the given struct for serialization. void QuicSession::GetLocalTransportParams(ngtcp2_transport_params* params) { - CHECK(!is_flag_set(QUICSESSION_FLAG_DESTROYED)); + CHECK(!is_destroyed()); ngtcp2_conn_get_local_transport_params(connection(), params); } // Gets the QUIC version negotiated for this QuicSession uint32_t QuicSession::negotiated_version() const { - CHECK(!is_flag_set(QUICSESSION_FLAG_DESTROYED)); + CHECK(!is_destroyed()); return ngtcp2_conn_get_negotiated_version(connection()); } @@ -328,7 +331,7 @@ void QuicSession::HandshakeConfirmed() { } bool QuicSession::is_handshake_completed() const { - DCHECK(!is_flag_set(QUICSESSION_FLAG_DESTROYED)); + DCHECK(!is_destroyed()); return ngtcp2_conn_get_handshake_completed(connection()); } @@ -342,7 +345,7 @@ void QuicSession::InitApplication() { // immediately closed without attempting to send any additional data to // the peer. All existing streams are abandoned and closed. void QuicSession::OnIdleTimeout() { - if (!is_flag_set(QUICSESSION_FLAG_DESTROYED)) { + if (!is_destroyed()) { state_[IDX_QUIC_SESSION_STATE_IDLE_TIMEOUT] = 1; Debug(this, "Idle timeout"); SilentClose(); @@ -359,7 +362,7 @@ void QuicSession::GetConnectionCloseInfo() { // Removes the given connection id from the QuicSession. void QuicSession::RemoveConnectionID(const QuicCID& cid) { - if (!is_flag_set(QUICSESSION_FLAG_DESTROYED)) + if (!is_destroyed()) DisassociateCID(cid); } @@ -440,24 +443,12 @@ SessionTicketAppData::Status QuicSession::GetSessionTicketAppData( return application_->GetSessionTicketAppData(app_data, flag); } -bool QuicSession::is_gracefully_closing() const { - return is_flag_set(QUICSESSION_FLAG_GRACEFUL_CLOSING); -} - -bool QuicSession::is_destroyed() const { - return is_flag_set(QUICSESSION_FLAG_DESTROYED); -} - -bool QuicSession::is_stateless_reset() const { - return is_flag_set(QUICSESSION_FLAG_STATELESS_RESET); -} - bool QuicSession::is_server() const { return crypto_context_->side() == NGTCP2_CRYPTO_SIDE_SERVER; } void QuicSession::StartGracefulClose() { - set_flag(QUICSESSION_FLAG_GRACEFUL_CLOSING); + set_graceful_closing(); RecordTimestamp(&QuicSessionStats::closing_at); } @@ -511,15 +502,15 @@ bool QuicSession::SendPacket( } void QuicSession::set_local_address(const ngtcp2_addr* addr) { - DCHECK(!is_flag_set(QUICSESSION_FLAG_DESTROYED)); + DCHECK(!is_destroyed()); ngtcp2_conn_set_local_addr(connection(), addr); } // Set the transport parameters received from the remote peer void QuicSession::set_remote_transport_params() { - DCHECK(!is_flag_set(QUICSESSION_FLAG_DESTROYED)); + DCHECK(!is_destroyed()); ngtcp2_conn_get_remote_transport_params(connection(), &transport_params_); - set_flag(QUICSESSION_FLAG_HAS_TRANSPORT_PARAMS); + set_transport_params_set(); } void QuicSession::StopIdleTimer() { @@ -537,7 +528,7 @@ void QuicSession::StopRetransmitTimer() { // parameter is an array of versions supported by the remote peer. void QuicSession::VersionNegotiation(const uint32_t* sv, size_t nsv) { CHECK(!is_server()); - if (!is_flag_set(QUICSESSION_FLAG_DESTROYED)) + if (!is_destroyed()) listener()->OnVersionNegotiation(NGTCP2_PROTO_VER, sv, nsv); } diff --git a/src/quic/node_quic_session.cc b/src/quic/node_quic_session.cc index d513b163cda7f1..d28f97cdba8fb8 100644 --- a/src/quic/node_quic_session.cc +++ b/src/quic/node_quic_session.cc @@ -648,8 +648,7 @@ void JSQuicSessionListener::OnSessionTicket(int size, SSL_SESSION* sess) { argv[0] = session_ticket.ToBuffer().ToLocalChecked(); } - if (session()->is_flag_set( - QuicSession::QUICSESSION_FLAG_HAS_TRANSPORT_PARAMS)) { + if (session()->is_transport_params_set()) { argv[1] = Buffer::Copy( env, reinterpret_cast(&session()->transport_params_), @@ -923,15 +922,9 @@ int QuicCryptoContext::OnClientHello() { TLSCallbackScope callback_scope(this); - // Not an error but does suspend the handshake until we're ready to go. - // A callback function is passed to the JavaScript function below that - // must be called in order to turn QUICSESSION_FLAG_CLIENT_HELLO_CB_RUNNING - // off. Once that callback is invoked, the TLS Handshake will resume. - // It is recommended that the user not take a long time to invoke the - // callback in order to avoid stalling out the QUIC connection. - if (in_client_hello_) + if (is_in_client_hello()) return -1; - in_client_hello_ = true; + set_in_client_hello(); QuicCryptoContext* ctx = session_->crypto_context(); session_->listener()->OnClientHello( @@ -943,7 +936,7 @@ int QuicCryptoContext::OnClientHello() { // handshake is ready to proceed. When the OnClientHello callback // is called above, it may be resolved synchronously or asynchronously. // In case it is resolved synchronously, we need the check below. - return in_client_hello_ ? -1 : 0; + return is_in_client_hello() ? -1 : 0; } // The OnCert callback provides an opportunity to prompt the server to @@ -965,9 +958,9 @@ int QuicCryptoContext::OnOCSP() { // As in node_crypto.cc, this is not an error, but does suspend the // handshake to continue when OnOCSP is complete. - if (in_ocsp_request_) + if (is_in_ocsp_request()) return -1; - in_ocsp_request_ = true; + set_in_ocsp_request(); session_->listener()->OnCert(session_->crypto_context()->servername()); @@ -975,7 +968,7 @@ int QuicCryptoContext::OnOCSP() { // request to be completed. When the OnCert handler is invoked // above, it can be resolve synchronously or asynchonously. If // resolved synchronously, we need the check below. - return in_ocsp_request_ ? -1 : 1; + return is_in_ocsp_request() ? -1 : 1; } // The OnCertDone function is called by the QuicSessionOnCertDone @@ -989,7 +982,9 @@ void QuicCryptoContext::OnOCSPDone( ocsp_response->IsArrayBufferView() ? "Yes" : "No"); // Continue the TLS handshake when this function exits // otherwise it will stall and fail. - TLSHandshakeScope handshake_scope(this, &in_ocsp_request_); + TLSHandshakeScope handshake_scope( + this, + [&]() { set_in_ocsp_request(false); }); // Disable the callback at this point so we don't loop continuously session_->state_[IDX_QUIC_SESSION_STATE_CERT_ENABLED] = 0; @@ -1137,9 +1132,9 @@ bool QuicCryptoContext::InitiateKeyUpdate() { // There's no user code that should be able to run while UpdateKey // is running, but we need to gate on it just to be safe. - auto leave = OnScopeLeave([&]() { in_key_update_ = false; }); - CHECK(!in_key_update_); - in_key_update_ = true; + auto leave = OnScopeLeave([&]() { set_in_key_update(false); }); + CHECK(!is_in_key_update()); + set_in_key_update(); Debug(session(), "Initiating Key Update"); session_->IncrementStat(&QuicSessionStats::keyupdate_count); @@ -1549,7 +1544,7 @@ void QuicSession::AckedStreamDataOffset( // It is possible for the QuicSession to have been destroyed but not yet // deconstructed. In such cases, we want to ignore the callback as there // is nothing to do but wait for further cleanup to happen. - if (LIKELY(!is_flag_set(QUICSESSION_FLAG_DESTROYED))) { + if (LIKELY(!is_destroyed())) { Debug(this, "Received acknowledgement for %" PRIu64 " bytes of stream %" PRId64 " data", @@ -1602,7 +1597,7 @@ void QuicSession::AddToSocket(QuicSocket* socket) { // Add the given QuicStream to this QuicSession's collection of streams. All // streams added must be removed before the QuicSession instance is freed. void QuicSession::AddStream(BaseObjectPtr stream) { - DCHECK(!is_flag_set(QUICSESSION_FLAG_GRACEFUL_CLOSING)); + DCHECK(!is_graceful_closing()); Debug(this, "Adding stream %" PRId64 " to session", stream->id()); streams_.emplace(stream->id(), stream); @@ -1642,9 +1637,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_flag_set(QUICSESSION_FLAG_CLOSING)) + if (is_closing() || is_silent_closing()) return; - set_flag(QUICSESSION_FLAG_CLOSING); + set_closing(); QuicError err = last_error(); Debug(this, "Immediate close with code %" PRIu64 " (%s)", @@ -1657,9 +1652,9 @@ void QuicSession::ImmediateClose() { // 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) { - CHECK(!is_flag_set(QUICSESSION_FLAG_DESTROYED)); - CHECK(!is_flag_set(QUICSESSION_FLAG_GRACEFUL_CLOSING)); - CHECK(!is_flag_set(QUICSESSION_FLAG_CLOSING)); + CHECK(!is_destroyed()); + CHECK(!is_graceful_closing()); + CHECK(!is_closing()); BaseObjectPtr stream = QuicStream::New(this, stream_id); CHECK(stream); @@ -1671,7 +1666,7 @@ BaseObjectPtr QuicSession::CreateStream(int64_t stream_id) { // the QuicSession instance will be generally unusable but most // likely will not be immediately freed. void QuicSession::Destroy() { - if (is_flag_set(QUICSESSION_FLAG_DESTROYED)) + if (is_destroyed()) return; // If we're not in the closing or draining periods, @@ -1689,9 +1684,9 @@ void QuicSession::Destroy() { CHECK(streams_.empty()); // Mark the session destroyed. - set_flag(QUICSESSION_FLAG_DESTROYED); - set_flag(QUICSESSION_FLAG_CLOSING, false); - set_flag(QUICSESSION_FLAG_GRACEFUL_CLOSING, false); + set_destroyed(); + set_closing(false); + set_graceful_closing(false); // Stop and free the idle and retransmission timers if they are active. StopIdleTimer(); @@ -1714,9 +1709,8 @@ bool QuicSession::GetNewConnectionID( ngtcp2_cid* cid, uint8_t* token, size_t cidlen) { - if (is_flag_set(QUICSESSION_FLAG_DESTROYED)) + if (is_destroyed()) return false; - CHECK(!is_flag_set(QUICSESSION_FLAG_DESTROYED)); CHECK_NOT_NULL(connection_id_strategy_); connection_id_strategy_(this, cid, cidlen); QuicCID cid_(cid); @@ -1739,7 +1733,7 @@ void QuicSession::HandleError() { // which determines whether or not we need to retransmit data to // to packet loss or ack delay. void QuicSession::MaybeTimeout() { - if (is_flag_set(QUICSESSION_FLAG_DESTROYED)) + if (is_destroyed()) return; uint64_t now = uv_hrtime(); bool transmit = false; @@ -1759,16 +1753,16 @@ void QuicSession::MaybeTimeout() { } bool QuicSession::OpenBidirectionalStream(int64_t* stream_id) { - DCHECK(!is_flag_set(QUICSESSION_FLAG_DESTROYED)); - DCHECK(!is_flag_set(QUICSESSION_FLAG_CLOSING)); - DCHECK(!is_flag_set(QUICSESSION_FLAG_GRACEFUL_CLOSING)); + DCHECK(!is_destroyed()); + DCHECK(!is_closing()); + DCHECK(!is_graceful_closing()); return ngtcp2_conn_open_bidi_stream(connection(), stream_id, nullptr) == 0; } bool QuicSession::OpenUnidirectionalStream(int64_t* stream_id) { - DCHECK(!is_flag_set(QUICSESSION_FLAG_DESTROYED)); - DCHECK(!is_flag_set(QUICSESSION_FLAG_CLOSING)); - DCHECK(!is_flag_set(QUICSESSION_FLAG_GRACEFUL_CLOSING)); + DCHECK(!is_destroyed()); + DCHECK(!is_closing()); + DCHECK(!is_graceful_closing()); if (ngtcp2_conn_open_uni_stream(connection(), stream_id, nullptr)) return false; ngtcp2_conn_shutdown_stream_read(connection(), *stream_id, 0); @@ -1808,8 +1802,8 @@ void QuicSession::PathValidation( // closing or draining period has started, this is a non-op. void QuicSession::Ping() { if (Ngtcp2CallbackScope::InNgtcp2CallbackScope(this) || - is_flag_set(QUICSESSION_FLAG_DESTROYED) || - is_flag_set(QUICSESSION_FLAG_CLOSING) || + is_destroyed() || + is_closing() || is_in_closing_period() || is_in_draining_period()) { return; @@ -1830,7 +1824,7 @@ bool QuicSession::Receive( const SocketAddress& local_addr, const SocketAddress& remote_addr, unsigned int flags) { - if (is_flag_set(QUICSESSION_FLAG_DESTROYED)) { + if (is_destroyed()) { Debug(this, "Ignoring packet because session is destroyed"); return false; } @@ -1840,7 +1834,7 @@ bool QuicSession::Receive( // Closing period starts once ngtcp2 has detected that the session // is being shutdown locally. Note that this is different that the - // is_flag_set(QUICSESSION_FLAG_GRACEFUL_CLOSING) function, which + // is_graceful_closing() function, which // indicates a graceful shutdown that allows the session and streams // to finish naturally. When is_in_closing_period is true, ngtcp2 is // actively in the process of shutting down the connection and a @@ -1944,7 +1938,7 @@ bool QuicSession::ReceivePacket( // If the QuicSession has been destroyed, we're not going // to process any more packets for it. - if (is_flag_set(QUICSESSION_FLAG_DESTROYED)) + if (is_destroyed()) return true; uint64_t now = uv_hrtime(); @@ -2001,7 +1995,7 @@ bool QuicSession::ReceiveStreamData( if (UNLIKELY(!(flags & NGTCP2_STREAM_DATA_FLAG_FIN) && datalen == 0)) return true; - if (is_flag_set(QUICSESSION_FLAG_DESTROYED)) + if (is_destroyed()) return false; HandleScope scope(env()->isolate()); @@ -2101,7 +2095,7 @@ bool QuicSession::SendConnectionClose() { // Do not send any frames at all if we're in the draining period // or in the middle of a silent close - if (is_in_draining_period() || is_flag_set(QUICSESSION_FLAG_SILENT_CLOSE)) + if (is_in_draining_period() || is_silent_closing()) return true; // The specific handling of connection close varies for client @@ -2204,7 +2198,7 @@ void QuicSession::UsePreferredAddressStrategy( // Passes a serialized packet to the associated QuicSocket. bool QuicSession::SendPacket(std::unique_ptr packet) { - CHECK(!is_flag_set(QUICSESSION_FLAG_DESTROYED)); + CHECK(!is_destroyed()); CHECK(!is_in_draining_period()); // There's nothing to send. @@ -2262,7 +2256,7 @@ void QuicSession::SendPendingData() { // session resumption. int QuicSession::set_session(SSL_SESSION* session) { CHECK(!is_server()); - CHECK(!is_flag_set(QUICSESSION_FLAG_DESTROYED)); + CHECK(!is_destroyed()); int size = i2d_SSL_SESSION(session, nullptr); if (size > SecureContext::kMaxSessionSize) return 0; @@ -2274,9 +2268,9 @@ int QuicSession::set_session(SSL_SESSION* session) { // TODO(@jasnell): This will be revisited. bool QuicSession::set_socket(QuicSocket* socket, bool nat_rebinding) { CHECK(!is_server()); - CHECK(!is_flag_set(QUICSESSION_FLAG_DESTROYED)); + CHECK(!is_destroyed()); - if (is_flag_set(QUICSESSION_FLAG_GRACEFUL_CLOSING)) + if (is_graceful_closing()) return false; if (socket == nullptr || socket == socket_.get()) @@ -2340,9 +2334,9 @@ void QuicSession::ResumeStream(int64_t stream_id) { // notify the JavaScript side and destroy the connection with // a flag set that indicates stateless reset. void QuicSession::SilentClose() { - CHECK(!is_flag_set(QUICSESSION_FLAG_SILENT_CLOSE)); - set_flag(QUICSESSION_FLAG_SILENT_CLOSE); - set_flag(QUICSESSION_FLAG_CLOSING); + CHECK(!is_silent_closing()); + set_silent_closing(); + set_closing(); QuicError err = last_error(); Debug(this, @@ -2364,7 +2358,7 @@ void QuicSession::SilentClose() { // multiple times. On client QuicSession instances, we only ever // serialize the connection close once. bool QuicSession::StartClosingPeriod() { - if (is_flag_set(QUICSESSION_FLAG_DESTROYED)) + if (is_destroyed()) return false; if (is_in_closing_period()) return true; @@ -2403,7 +2397,7 @@ bool QuicSession::StartClosingPeriod() { // Called by ngtcp2 when a stream has been closed. If the stream does // not exist, the close is ignored. void QuicSession::StreamClose(int64_t stream_id, uint64_t app_error_code) { - if (is_flag_set(QUICSESSION_FLAG_DESTROYED)) + if (is_destroyed()) return; Debug(this, "Closing stream %" PRId64 " with code %" PRIu64, @@ -2419,7 +2413,7 @@ void QuicSession::StreamClose(int64_t stream_id, uint64_t app_error_code) { // a stream commitment attack. The only exception is shutting the // stream down explicitly if we are in a graceful close period. void QuicSession::StreamOpen(int64_t stream_id) { - if (is_flag_set(QUICSESSION_FLAG_GRACEFUL_CLOSING)) { + if (is_graceful_closing()) { ngtcp2_conn_shutdown_stream( connection(), stream_id, @@ -2451,7 +2445,7 @@ void QuicSession::StreamReset( int64_t stream_id, uint64_t final_size, uint64_t app_error_code) { - if (is_flag_set(QUICSESSION_FLAG_DESTROYED)) + if (is_destroyed()) return; Debug(this, @@ -2514,7 +2508,7 @@ void QuicSession::UpdateIdleTimer() { // serialize stream data that is being sent initially. bool QuicSession::WritePackets(const char* diagnostic_label) { CHECK(!Ngtcp2CallbackScope::InNgtcp2CallbackScope(this)); - CHECK(!is_flag_set(QUICSESSION_FLAG_DESTROYED)); + CHECK(!is_destroyed()); // During the draining period, we must not send any frames at all. if (is_in_draining_period()) @@ -3269,7 +3263,7 @@ int QuicSession::OnStatelessReset( QuicSession* session = static_cast(user_data); if (UNLIKELY(session->is_destroyed())) return NGTCP2_ERR_CALLBACK_FAILURE; - session->set_flag(QUICSESSION_FLAG_STATELESS_RESET); + session->set_stateless_reset(); return 0; } diff --git a/src/quic/node_quic_session.h b/src/quic/node_quic_session.h index 1e9341beecfe05..8c22fe3bdab534 100644 --- a/src/quic/node_quic_session.h +++ b/src/quic/node_quic_session.h @@ -358,6 +358,13 @@ class JSQuicSessionListener : public QuicSessionListener { friend class QuicSession; }; +#define QUICCRYPTOCONTEXT_FLAGS(V) \ + V(IN_TLS_CALLBACK, in_tls_callback) \ + V(IN_KEY_UPDATE, in_key_update) \ + V(IN_OCSP_RESPONSE, in_ocsp_request) \ + V(IN_CLIENT_HELLO, in_client_hello) \ + V(EARLY_DATA, early_data) + // The QuicCryptoContext class encapsulates all of the crypto/TLS // handshake details on behalf of a QuicSession. class QuicCryptoContext : public MemoryRetainer { @@ -440,6 +447,18 @@ class QuicCryptoContext : public MemoryRetainer { options_ &= ~option; } +#define V(id, name) \ + inline bool is_##name() const { \ + return flags_ & (1 << QUICCRYPTOCONTEXT_FLAG_##id); } \ + inline void set_##name(bool on = true) { \ + if (on) \ + flags_ |= (1 << QUICCRYPTOCONTEXT_FLAG_##id); \ + else \ + flags_ &= ~(1 << QUICCRYPTOCONTEXT_FLAG_##id); \ + } + QUICCRYPTOCONTEXT_FLAGS(V) +#undef V + inline bool set_session(crypto::SSLSessionPointer session); inline void set_tls_alert(int err); @@ -480,29 +499,32 @@ class QuicCryptoContext : public MemoryRetainer { ngtcp2_crypto_side side_; crypto::SSLPointer ssl_; QuicBuffer handshake_[3]; - bool in_tls_callback_ = false; - bool in_key_update_ = false; - bool in_ocsp_request_ = false; - bool in_client_hello_ = false; - bool early_data_ = false; uint32_t options_; + uint32_t flags_ = 0; v8::Global ocsp_response_; crypto::BIOPointer bio_trace_; +#define V(id, _) QUICCRYPTOCONTEXT_FLAG_##id, + enum QuicCryptoContextFlags : uint32_t { + QUICCRYPTOCONTEXT_FLAGS(V) + QUICCRYPTOCONTEXT_FLAG_COUNT + }; +#undef V + class TLSCallbackScope { public: explicit TLSCallbackScope(QuicCryptoContext* context) : context_(context) { - context_->in_tls_callback_ = true; + context_->set_in_tls_callback(); } ~TLSCallbackScope() { - context_->in_tls_callback_ = false; + context_->set_in_tls_callback(false); } static bool is_in_callback(QuicCryptoContext* context) { - return context->in_tls_callback_; + return context->is_in_tls_callback(); } private: @@ -511,17 +533,18 @@ class QuicCryptoContext : public MemoryRetainer { class TLSHandshakeScope { public: + using DoneCB = std::function; TLSHandshakeScope( QuicCryptoContext* context, - bool* monitor) : + DoneCB done) : context_(context), - monitor_(monitor) {} + done_(done) {} ~TLSHandshakeScope() { if (!is_handshake_suspended()) return; - *monitor_ = false; + done_(); // Only continue the TLS handshake if we are not currently running // synchronously within the TLS handshake function. This can happen // when the callback function passed to the clientHello and cert @@ -533,12 +556,12 @@ class QuicCryptoContext : public MemoryRetainer { private: bool is_handshake_suspended() const { - return context_->in_ocsp_request_ || context_->in_client_hello_; + return context_->is_in_ocsp_request() || context_->is_in_client_hello(); } QuicCryptoContext* context_; - bool* monitor_; + DoneCB done_; }; friend class QuicSession; @@ -663,6 +686,17 @@ class QuicApplication : public MemoryRetainer, size_t max_header_length_ = 0; }; +// QUICSESSION_FLAGS are converted into is_{name}() and set_{name}(bool on) +// accessors on the QuicSession class. +#define QUICSESSION_FLAGS(V) \ + V(CLOSING, closing) \ + V(GRACEFUL_CLOSING, graceful_closing) \ + V(DESTROYED, destroyed) \ + V(TRANSPORT_PARAMS_SET, transport_params_set) \ + V(NGTCP2_CALLBACK, in_ngtcp2_callback) \ + V(SILENT_CLOSE, silent_closing) \ + V(STATELESS_RESET, stateless_reset) + // The QuicSession class is an virtual class that serves as // the basis for both client and server QuicSession. // It implements the functionality that is shared for both @@ -801,15 +835,14 @@ class QuicSession : public AsyncWrap, inline bool allow_early_data() const; - // Returns true if StartGracefulClose() has been called and the - // QuicSession is currently in the process of a graceful close. - inline bool is_gracefully_closing() const; - - // Returns true if Destroy() has been called and the - // QuicSession is no longer usable. - inline bool is_destroyed() const; - - inline bool is_stateless_reset() const; +#define V(id, name) \ + bool is_##name() const { return flags_ & (1 << QUICSESSION_FLAG_##id); } \ + void set_##name(bool on = true) { \ + if (on) flags_ |= (1 << QUICSESSION_FLAG_##id); \ + else flags_ &= ~(1 << QUICSESSION_FLAG_##id); \ + } + QUICSESSION_FLAGS(V) +#undef V // Returns true if the QuicSession has entered the // closing period following a call to ImmediateClose. @@ -1089,15 +1122,15 @@ class QuicSession : public AsyncWrap, explicit Ngtcp2CallbackScope(QuicSession* session) : session_(session) { CHECK(session_); CHECK(!InNgtcp2CallbackScope(session)); - session_->set_flag(QUICSESSION_FLAG_NGTCP2_CALLBACK); + session_->set_in_ngtcp2_callback(); } ~Ngtcp2CallbackScope() { - session_->set_flag(QUICSESSION_FLAG_NGTCP2_CALLBACK, false); + session_->set_in_ngtcp2_callback(false); } static bool InNgtcp2CallbackScope(QuicSession* session) { - return session->is_flag_set(QUICSESSION_FLAG_NGTCP2_CALLBACK); + return session->is_in_ngtcp2_callback(); } private: @@ -1356,55 +1389,12 @@ class QuicSession : public AsyncWrap, bool StartClosingPeriod(); +#define V(id, _) QUICSESSION_FLAG_##id, enum QuicSessionFlags : uint32_t { - // Initial state when a QuicSession is created but nothing yet done. - QUICSESSION_FLAG_INITIAL = 0x1, - - // Set while the QuicSession is in the process of an Immediate - // or silent close. - QUICSESSION_FLAG_CLOSING = 0x2, - - // Set while the QuicSession is in the process of a graceful close. - QUICSESSION_FLAG_GRACEFUL_CLOSING = 0x4, - - // Set when the QuicSession has been destroyed (but not - // yet freed) - QUICSESSION_FLAG_DESTROYED = 0x8, - - QUICSESSION_FLAG_HAS_TRANSPORT_PARAMS = 0x10, - - // Set while the QuicSession is executing an ngtcp2 callback - QUICSESSION_FLAG_NGTCP2_CALLBACK = 0x100, - - // Set if the QuicSession is in the middle of a silent close - // (that is, a CONNECTION_CLOSE should not be sent) - QUICSESSION_FLAG_SILENT_CLOSE = 0x200, - - QUICSESSION_FLAG_HANDSHAKE_RX = 0x400, - QUICSESSION_FLAG_HANDSHAKE_TX = 0x800, - QUICSESSION_FLAG_HANDSHAKE_KEYS = - QUICSESSION_FLAG_HANDSHAKE_RX | - QUICSESSION_FLAG_HANDSHAKE_TX, - QUICSESSION_FLAG_SESSION_RX = 0x1000, - QUICSESSION_FLAG_SESSION_TX = 0x2000, - QUICSESSION_FLAG_SESSION_KEYS = - QUICSESSION_FLAG_SESSION_RX | - QUICSESSION_FLAG_SESSION_TX, - - // Set if the QuicSession was closed due to stateless reset - QUICSESSION_FLAG_STATELESS_RESET = 0x4000 + QUICSESSION_FLAGS(V) + QUICSESSION_FLAG_COUNT }; - - void set_flag(QuicSessionFlags flag, bool on = true) { - if (on) - flags_ |= flag; - else - flags_ &= ~flag; - } - - bool is_flag_set(QuicSessionFlags flag) const { - return (flags_ & flag) == flag; - } +#undef V void IncrementConnectionCloseAttempts() { if (connection_close_attempts_ < kMaxSizeT) diff --git a/src/quic/node_quic_socket-inl.h b/src/quic/node_quic_socket-inl.h index 38f3ad927180f0..bcc85d82314f22 100644 --- a/src/quic/node_quic_socket-inl.h +++ b/src/quic/node_quic_socket-inl.h @@ -96,9 +96,9 @@ void QuicSocket::DisassociateStatelessResetToken( // existing sessions are allowed to close naturally but new // sessions are rejected. void QuicSocket::StopListening() { - if (is_flag_set(QUICSOCKET_FLAGS_SERVER_LISTENING)) { + if (is_server_listening()) { Debug(this, "Stop listening"); - set_flag(QUICSOCKET_FLAGS_SERVER_LISTENING, false); + set_server_listening(false); // It is important to not call ReceiveStop here as there // is ongoing traffic being exchanged by the peers. } @@ -155,9 +155,9 @@ size_t QuicSocket::GetCurrentStatelessResetCounter(const SocketAddress& addr) { return it == std::end(reset_counts_) ? 0 : it->second; } -void QuicSocket::set_server_busy(bool on) { +void QuicSocket::ServerBusy(bool on) { Debug(this, "Turning Server Busy Response %s", on ? "on" : "off"); - set_flag(QUICSOCKET_FLAGS_SERVER_BUSY, on); + set_server_busy(on); listener_->OnServerBusy(on); } @@ -174,14 +174,12 @@ void QuicSocket::set_diagnostic_packet_loss(double rx, double tx) { } bool QuicSocket::ToggleStatelessReset() { - set_flag( - QUICSOCKET_FLAGS_DISABLE_STATELESS_RESET, - !is_flag_set(QUICSOCKET_FLAGS_DISABLE_STATELESS_RESET)); - return !is_flag_set(QUICSOCKET_FLAGS_DISABLE_STATELESS_RESET); + set_stateless_reset_disabled(!is_stateless_reset_disabled()); + return !is_stateless_reset_disabled(); } void QuicSocket::set_validated_address(const SocketAddress& addr) { - if (is_option_set(QUICSOCKET_OPTIONS_VALIDATE_ADDRESS_LRU)) { + if (has_option_validate_address_lru()) { // Remove the oldest item if we've hit the LRU limit validated_addrs_.push_back(SocketAddress::Hash()(addr)); if (validated_addrs_.size() > kMaxValidateAddressLru) @@ -190,7 +188,7 @@ void QuicSocket::set_validated_address(const SocketAddress& addr) { } bool QuicSocket::is_validated_address(const SocketAddress& addr) const { - if (is_option_set(QUICSOCKET_OPTIONS_VALIDATE_ADDRESS_LRU)) { + if (has_option_validate_address_lru()) { auto res = std::find(std::begin(validated_addrs_), std::end(validated_addrs_), SocketAddress::Hash()(addr)); @@ -217,7 +215,7 @@ void QuicSocket::AddEndpoint( if (preferred || endpoints_.empty()) preferred_endpoint_ = endpoint_; endpoints_.emplace_back(endpoint_); - if (is_flag_set(QUICSOCKET_FLAGS_SERVER_LISTENING)) + if (is_server_listening()) endpoint_->ReceiveStart(); } diff --git a/src/quic/node_quic_socket.cc b/src/quic/node_quic_socket.cc index e3112887d8e4fb..6ebdb30ef8a250 100644 --- a/src/quic/node_quic_socket.cc +++ b/src/quic/node_quic_socket.cc @@ -267,7 +267,7 @@ QuicSocket::QuicSocket( EntropySource(token_secret_, kTokenSecretLen); if (disable_stateless_reset) - set_flag(QUICSOCKET_FLAGS_DISABLE_STATELESS_RESET); + set_stateless_reset_disabled(); // Set the session reset secret to the one provided or random. // Note that a random secret is going to make it exceedingly @@ -316,13 +316,13 @@ void QuicSocket::Listen( const std::string& alpn, uint32_t options) { CHECK(sc); - CHECK(!is_flag_set(QUICSOCKET_FLAGS_SERVER_LISTENING)); + CHECK(!is_server_listening()); Debug(this, "Starting to listen"); server_session_config_.Set(quic_state(), preferred_address); server_secure_context_ = sc; server_alpn_ = alpn; server_options_ = options; - set_flag(QUICSOCKET_FLAGS_SERVER_LISTENING); + set_server_listening(); RecordTimestamp(&QuicSocketStats::listen_at); ReceiveStart(); } @@ -737,7 +737,7 @@ BaseObjectPtr QuicSocket::AcceptInitialPacket( QuicCID ocid; // If the QuicSocket is not listening, the paket will be ignored. - if (!is_flag_set(QUICSOCKET_FLAGS_SERVER_LISTENING)) { + if (!is_server_listening()) { Debug(this, "QuicSocket is not listening"); return {}; } @@ -762,7 +762,7 @@ BaseObjectPtr QuicSocket::AcceptInitialPacket( // Else, check to see if the number of connections total for this QuicSocket // has been exceeded. If the count has been exceeded, shutdown the connection // immediately after the initial keys are installed. - if (UNLIKELY(is_flag_set(QUICSOCKET_FLAGS_SERVER_BUSY)) || + if (UNLIKELY(is_server_busy()) || sessions_.size() >= max_connections_ || GetCurrentSocketAddressCounter(remote_addr) >= max_connections_per_host_) { @@ -786,8 +786,7 @@ BaseObjectPtr QuicSocket::AcceptInitialPacket( if (!is_validated_address(remote_addr)) { switch (hd.type) { case NGTCP2_PKT_INITIAL: - if (is_option_set(QUICSOCKET_OPTIONS_VALIDATE_ADDRESS) || - hd.token.len > 0) { + if (has_option_validate_address() || hd.token.len > 0) { Debug(this, "Performing explicit address validation"); if (hd.token.len == 0) { Debug(this, "No retry token was detected. Generating one"); @@ -1124,7 +1123,7 @@ void QuicSocketSetServerBusy(const FunctionCallbackInfo& args) { QuicSocket* socket; ASSIGN_OR_RETURN_UNWRAP(&socket, args.Holder()); CHECK_EQ(args.Length(), 1); - socket->set_server_busy(args[0]->IsTrue()); + socket->ServerBusy(args[0]->IsTrue()); } void QuicSocketToggleStatelessReset(const FunctionCallbackInfo& args) { diff --git a/src/quic/node_quic_socket.h b/src/quic/node_quic_socket.h index 2fcf0fb7f19d31..a9d4058fb7a1e0 100644 --- a/src/quic/node_quic_socket.h +++ b/src/quic/node_quic_socket.h @@ -36,17 +36,23 @@ namespace quic { class QuicSocket; class QuicEndpoint; +#define QUICSOCKET_OPTIONS(V) \ + V(VALIDATE_ADDRESS, validate_address) \ + V(VALIDATE_ADDRESS_LRU, validate_address_lru) + +#define V(id, _) QUICSOCKET_OPTIONS_##id, enum QuicSocketOptions : uint32_t { - // When enabled the QuicSocket will validate the address - // using a RETRY packet to the peer. - QUICSOCKET_OPTIONS_VALIDATE_ADDRESS = 0x1, - - // When enabled, and the VALIDATE_ADDRESS option is also - // set, the QuicSocket will use an LRU cache to track - // validated addresses. Address validation will be skipped - // if the address is currently in the cache. - QUICSOCKET_OPTIONS_VALIDATE_ADDRESS_LRU = 0x2, + QUICSOCKET_OPTIONS(V) + QUICSOCKET_OPTIONS_COUNT }; +#undef V + +#define QUICSOCKET_FLAGS(V) \ + V(GRACEFUL_CLOSE, graceful_closing) \ + V(WAITING_FOR_CALLBACKS, waiting_for_callbacks) \ + V(SERVER_LISTENING, server_listening) \ + V(SERVER_BUSY, server_busy) \ + V(DISABLE_STATELESS_RESET, stateless_reset_disabled) #define SOCKET_STATS(V) \ V(CREATED_AT, created_at, "Created At") \ @@ -351,7 +357,25 @@ class QuicSocket : public AsyncWrap, std::unique_ptr packet, BaseObjectPtr session = BaseObjectPtr()); - inline void set_server_busy(bool on); +#define V(id, name) \ + inline bool is_##name() const { \ + return flags_ & (1 << QUICSOCKET_FLAG_##id); } \ + inline void set_##name(bool on = true) { \ + if (on) \ + flags_ |= (1 << QUICSOCKET_FLAG_##id); \ + else \ + flags_ &= ~(1 << QUICSOCKET_FLAG_##id); \ + } + QUICSOCKET_FLAGS(V) +#undef V + +#define V(id, name) \ + bool has_option_##name() const { \ + return options_ & (1 << QUICSOCKET_OPTIONS_##id); } + QUICSOCKET_OPTIONS(V) +#undef V + + inline void ServerBusy(bool on); inline void set_diagnostic_packet_loss(double rx = 0.0, double tx = 0.0); @@ -489,43 +513,12 @@ class QuicSocket : public AsyncWrap, // and the current packet should be artificially considered lost. inline bool is_diagnostic_packet_loss(double prob) const; - bool is_stateless_reset_disabled() { - return is_flag_set(QUICSOCKET_FLAGS_DISABLE_STATELESS_RESET); - } - +#define V(id, _) QUICSOCKET_FLAG_##id, enum QuicSocketFlags : uint32_t { - QUICSOCKET_FLAGS_NONE = 0x0, - - // Indicates that the QuicSocket has entered a graceful - // closing phase, indicating that no additional - QUICSOCKET_FLAGS_GRACEFUL_CLOSE = 0x1, - QUICSOCKET_FLAGS_WAITING_FOR_CALLBACKS = 0x2, - QUICSOCKET_FLAGS_SERVER_LISTENING = 0x4, - QUICSOCKET_FLAGS_SERVER_BUSY = 0x8, - QUICSOCKET_FLAGS_DISABLE_STATELESS_RESET = 0x10 + QUICSOCKET_FLAGS(V) + QUICSOCKET_FLAG_COUNT }; - - void set_flag(QuicSocketFlags flag, bool on = true) { - if (on) - flags_ |= flag; - else - flags_ &= ~flag; - } - - bool is_flag_set(QuicSocketFlags flag) const { - return flags_ & flag; - } - - void set_option(QuicSocketOptions option, bool on = true) { - if (on) - options_ |= option; - else - options_ &= ~option; - } - - bool is_option_set(QuicSocketOptions option) const { - return options_ & option; - } +#undef V ngtcp2_mem alloc_info_; @@ -533,8 +526,8 @@ class QuicSocket : public AsyncWrap, SocketAddress::Map> bound_endpoints_; BaseObjectWeakPtr preferred_endpoint_; - uint32_t flags_ = QUICSOCKET_FLAGS_NONE; - uint32_t options_; + uint32_t flags_ = 0; + uint32_t options_ = 0; uint32_t server_options_; size_t max_connections_ = DEFAULT_MAX_CONNECTIONS; diff --git a/src/quic/node_quic_stream-inl.h b/src/quic/node_quic_stream-inl.h index 546e867ee2db7e..e253875c4d3389 100644 --- a/src/quic/node_quic_stream-inl.h +++ b/src/quic/node_quic_stream-inl.h @@ -23,31 +23,16 @@ QuicStreamOrigin QuicStream::origin() const { QUIC_STREAM_CLIENT; } -bool QuicStream::is_flag_set(int32_t flag) const { - return flags_ & (1 << flag); -} - -void QuicStream::set_flag(int32_t flag, bool on) { - if (on) - flags_ |= (1 << flag); - else - flags_ &= ~(1 << flag); -} - void QuicStream::set_final_size(uint64_t final_size) { // Only set the final size once. - if (is_flag_set(QUICSTREAM_FLAG_FIN)) { + if (is_fin()) { CHECK_LE(final_size, GetStat(&QuicStreamStats::final_size)); return; } - set_flag(QUICSTREAM_FLAG_FIN); + set_fin(true); SetStat(&QuicStreamStats::final_size, final_size); } -bool QuicStream::is_destroyed() const { - return is_flag_set(QUICSTREAM_FLAG_DESTROYED); -} - bool QuicStream::was_ever_writable() const { if (direction() == QUIC_STREAM_UNIDIRECTIONAL) { return session_->is_server() ? @@ -72,17 +57,11 @@ bool QuicStream::was_ever_readable() const { } bool QuicStream::is_readable() const { - return was_ever_readable() && !is_flag_set(QUICSTREAM_FLAG_READ_CLOSED); -} - -void QuicStream::set_fin_sent() { - CHECK(!is_writable()); - set_flag(QUICSTREAM_FLAG_FIN_SENT); + return was_ever_readable() && !is_read_closed(); } bool QuicStream::is_write_finished() const { - return is_flag_set(QUICSTREAM_FLAG_FIN_SENT) && - streambuf_.length() == 0; + return is_fin_sent() && streambuf_.length() == 0; } bool QuicStream::SubmitInformation(v8::Local headers) { @@ -143,7 +122,7 @@ void QuicStream::ResetStream(uint64_t app_error_code) { session()->connection(), stream_id_, app_error_code); - set_flag(QUICSTREAM_FLAG_READ_CLOSED); + set_read_closed(); streambuf_.Cancel(); streambuf_.End(); } @@ -156,7 +135,7 @@ void QuicStream::StopSending(uint64_t app_error_code) { session()->connection(), stream_id_, app_error_code); - set_flag(QUICSTREAM_FLAG_READ_CLOSED); + set_read_closed(); } void QuicStream::Schedule(Queue* queue) { diff --git a/src/quic/node_quic_stream.cc b/src/quic/node_quic_stream.cc index ba447e760d206a..3b4e7286945957 100644 --- a/src/quic/node_quic_stream.cc +++ b/src/quic/node_quic_stream.cc @@ -124,8 +124,8 @@ void QuicStream::Destroy(QuicError* error) { QuicSession::SendSessionScope send_scope(session()); - set_flag(QUICSTREAM_FLAG_DESTROYED); - set_flag(QUICSTREAM_FLAG_READ_CLOSED); + set_read_closed(); + set_destroyed(); // In case this stream is scheduled for sending, remove it // from the schedule queue @@ -234,8 +234,8 @@ bool QuicStream::IsClosing() { int QuicStream::ReadStart() { CHECK(!is_destroyed()); CHECK(is_readable()); - set_flag(QUICSTREAM_FLAG_READ_STARTED); - set_flag(QUICSTREAM_FLAG_READ_PAUSED, false); + set_read_started(); + set_read_paused(false); IncrementStat( &QuicStreamStats::max_offset, inbound_consumed_data_while_paused_); @@ -246,7 +246,7 @@ int QuicStream::ReadStart() { int QuicStream::ReadStop() { CHECK(!is_destroyed()); CHECK(is_readable()); - set_flag(QUICSTREAM_FLAG_READ_PAUSED); + set_read_paused(); return 0; } @@ -348,7 +348,7 @@ void QuicStream::ReceiveData( datalen -= avail; // Capture read_paused before EmitRead in case user code callbacks // alter the state when EmitRead is called. - bool read_paused = is_flag_set(QUICSTREAM_FLAG_READ_PAUSED); + bool read_paused = is_read_paused(); EmitRead(avail, buf); // Reading can be paused while we are processing. If that's // the case, we still want to acknowledge the current bytes diff --git a/src/quic/node_quic_stream.h b/src/quic/node_quic_stream.h index 0d0809c74df0ed..53a8cdaea52d51 100644 --- a/src/quic/node_quic_stream.h +++ b/src/quic/node_quic_stream.h @@ -78,30 +78,13 @@ struct QuicStreamStatsTraits { static void ToString(const Base& ptr, Fn&& add_field); }; -enum QuicStreamStates : uint32_t { - // QuicStream is fully open. Readable and Writable - QUICSTREAM_FLAG_INITIAL = 0, - - // QuicStream Read State is closed because a final stream frame - // has been received from the peer or the QuicStream is unidirectional - // outbound only (i.e. it was never readable) - QUICSTREAM_FLAG_READ_CLOSED, - - // JavaScript side has switched into flowing mode (Readable side) - QUICSTREAM_FLAG_READ_STARTED, - - // JavaScript side has paused the flow of data (Readable side) - QUICSTREAM_FLAG_READ_PAUSED, - - // QuicStream has received a final stream frame (Readable side) - QUICSTREAM_FLAG_FIN, - - // QuicStream has sent a final stream frame (Writable side) - QUICSTREAM_FLAG_FIN_SENT, - - // QuicStream has been destroyed - QUICSTREAM_FLAG_DESTROYED -}; +#define QUICSTREAM_FLAGS(V) \ + V(READ_CLOSED, read_closed) \ + V(READ_STARTED, read_started) \ + V(READ_PAUSED, read_paused) \ + V(FIN, fin) \ + V(FIN_SENT, fin_sent) \ + V(DESTROYED, destroyed) enum QuicStreamDirection { // The QuicStream is readable and writable in both directions @@ -228,9 +211,6 @@ class QuicStream : public AsyncWrap, // or the server. inline QuicStreamOrigin origin() const; - // The QuicStream has been destroyed and is no longer usable. - inline bool is_destroyed() const; - // A QuicStream will not be writable if: // - The streambuf_ is ended // - It is a Unidirectional stream originating from the peer @@ -241,13 +221,6 @@ class QuicStream : public AsyncWrap, // - It is a Unidirectional stream originating from the local peer. inline bool is_readable() const; - // Records the fact that a final stream frame has been - // serialized and sent to the peer. There still may be - // unacknowledged data in the outbound queue, but no - // additional frames may be sent for the stream other - // than reset stream. - inline void set_fin_sent(); - // IsWriteFinished will return true if a final stream frame // has been sent and all data has been acknowledged (the // send buffer is empty). @@ -342,6 +315,18 @@ class QuicStream : public AsyncWrap, // Required for StreamBase int ReadStop() override; +#define V(id, name) \ + inline bool is_##name() const { \ + return flags_ & (1 << QUICSTREAM_FLAG_##id); } \ + inline void set_##name(bool on = true) { \ + if (on) \ + flags_ |= (1 << QUICSTREAM_FLAG_##id); \ + else \ + flags_ &= ~(1 << QUICSTREAM_FLAG_##id); \ + } + QUICSTREAM_FLAGS(V) +#undef V + // Required for StreamBase int DoShutdown(ShutdownWrap* req_wrap) override; @@ -363,10 +348,6 @@ class QuicStream : public AsyncWrap, size_t max_count_hint) override; private: - inline bool is_flag_set(int32_t flag) const; - - inline void set_flag(int32_t flag, bool on = true); - // WasEverWritable returns true if it is a bidirectional stream, // or a Unidirectional stream originating from the local peer. // If was_ever_writable() is false, then no stream frames should @@ -380,12 +361,20 @@ class QuicStream : public AsyncWrap, void IncrementStats(size_t datalen); +#define V(id, _) QUICSTREAM_FLAG_##id, + enum QuicStreamStates : uint32_t { + QUICSTREAM_FLAGS(V) + QUICSTREAM_FLAG_COUNT + }; +#undef V + BaseObjectWeakPtr session_; QuicBuffer streambuf_; int64_t stream_id_ = 0; int64_t push_id_ = 0; - uint32_t flags_ = QUICSTREAM_FLAG_INITIAL; + uint32_t flags_ = 0; + size_t inbound_consumed_data_while_paused_ = 0; std::vector> headers_; From 790eac1dd0e8c661cc23fdac6178eccdd3322217 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Wed, 1 Jul 2020 09:39:22 -0700 Subject: [PATCH 03/11] quic: additional cleanups on the c++ side --- 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); From a545951359cecaf21be18700d6d0b3ee89de5408 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Wed, 1 Jul 2020 15:06:27 -0700 Subject: [PATCH 04/11] quic: refactor QuicSession close/destroy flow --- lib/internal/quic/core.js | 121 ++++++------------ src/quic/node_quic_session-inl.h | 2 +- src/quic/node_quic_session.cc | 211 ++++++++++++------------------- src/quic/node_quic_session.h | 67 +++++++--- src/quic/node_quic_socket.cc | 2 + 5 files changed, 174 insertions(+), 229 deletions(-) diff --git a/lib/internal/quic/core.js b/lib/internal/quic/core.js index 90727194246450..75f07c8d7e5871 100644 --- a/lib/internal/quic/core.js +++ b/lib/internal/quic/core.js @@ -287,24 +287,10 @@ function onSessionReady(handle) { process.nextTick(emit.bind(socket, 'session', session)); } -// Called when the session needs to be closed and destroyed. -// If silent is true, then the session is going to be closed -// immediately without sending any CONNECTION_CLOSE to the -// connected peer. If silent is false, a CONNECTION_CLOSE -// is going to be sent to the peer. +// Called when the C++ QuicSession::Close() method has been called. +// Synchronously cleanup and destroy the JavaScript QuicSession. function onSessionClose(code, family, silent, statelessReset) { - if (this[owner_symbol]) { - if (silent) { - this[owner_symbol][kDestroy](statelessReset, family, code); - } else { - this[owner_symbol][kClose](family, code); - } - return; - } - // When there's no owner_symbol, the session was closed - // before it could be fully set up. Just immediately - // close everything down on the native side. - this.destroy(code, family); + this[owner_symbol][kDestroy](code, family, silent, statelessReset); } // Called by the C++ internals when a QuicSession has been destroyed. @@ -1654,6 +1640,7 @@ class QuicSession extends EventEmitter { maxPacketLength: NGTCP2_DEFAULT_MAX_PKTLEN, servername: undefined, socket: undefined, + silentClose: false, statelessReset: false, stats: undefined, pendingStreams: new Set(), @@ -1736,46 +1723,15 @@ class QuicSession extends EventEmitter { // Causes the QuicSession to be immediately destroyed, but with // additional metadata set. - [kDestroy](statelessReset, family, code) { + [kDestroy](code, family, silent, statelessReset) { const state = this[kInternalState]; - state.statelessReset = statelessReset; state.closeCode = code; state.closeFamily = family; + state.silentClose = silent; + state.statelessReset = statelessReset; this.destroy(); } - // Immediate close has been initiated for the session. Any - // still open QuicStreams must be abandoned and shutdown - // with RESET_STREAM and STOP_SENDING frames transmitted - // as appropriate. Once the streams have been shutdown, a - // CONNECTION_CLOSE will be generated and sent, switching - // the session into the closing period. - [kClose](family, code) { - const state = this[kInternalState]; - // Do nothing if the QuicSession has already been destroyed. - if (state.destroyed) - return; - - // Set the close code and family so we can keep track. - state.closeCode = code; - state.closeFamily = family; - - // Shutdown all pending streams. These are Streams that - // have been created but do not yet have a handle assigned. - for (const stream of state.pendingStreams) - stream[kClose](family, code); - - // Shutdown all of the remaining streams - for (const stream of state.streams.values()) - stream[kClose](family, code); - - // By this point, all necessary RESET_STREAM and - // STOP_SENDING frames ought to have been sent, - // so now we just trigger sending of the - // CONNECTION_CLOSE frame. - this[kHandle].close(family, code); - } - // Closes the specified stream with the given code. The // QuicStream object will be destroyed. [kStreamClose](id, code) { @@ -1873,14 +1829,6 @@ class QuicSession extends EventEmitter { this[kInternalState].streams.set(id, stream); } - // The QuicSession will be destroyed if closing has been - // called and there are no remaining streams - [kMaybeDestroy]() { - const state = this[kInternalState]; - if (state.closing && state.streams.size === 0) - this.destroy(); - } - // Called when a client QuicSession has opted to use the // server provided preferred address. This is a purely // informationational notification. It is not called on @@ -1890,6 +1838,17 @@ class QuicSession extends EventEmitter { emit.bind(this, 'usePreferredAddress', address)); } + // The QuicSession will be destroyed if close() has been + // called and there are no remaining streams + [kMaybeDestroy]() { + const state = this[kInternalState]; + if (state.closing && state.streams.size === 0) { + this.destroy(); + return true; + } + return false; + } + // Closing allows any existing QuicStream's to complete // normally but disallows any new QuicStreams from being // opened. Calls to openStream() will fail, and new streams @@ -1910,27 +1869,27 @@ class QuicSession extends EventEmitter { // has been destroyed if (state.closing) return; - state.closing = true; - this[kHandle].gracefulClose(); - // See if we can close immediately. - this[kMaybeDestroy](); + // If there are no active streams, we can close immediately, + // otherwise set the graceful close flag. + if (!this[kMaybeDestroy]()) + this[kHandle].gracefulClose(); } // Destroying synchronously shuts down and frees the // QuicSession immediately, even if there are still open // streams. // - // A CONNECTION_CLOSE packet is sent to the - // connected peer and the session is immediately - // destroyed. + // Unless we're in the middle of a silent close, a + // CONNECTION_CLOSE packet will be sent to the connected + // peer and the session will be immediately destroyed. // // If destroy is called with an error argument, the // 'error' event is emitted on next tick. // // Once destroyed, and after the 'error' event (if any), - // the close event is emitted on next tick. + // the 'close' event is emitted on next tick. destroy(error) { const state = this[kInternalState]; // Destroy can only be called once. Multiple calls will be ignored @@ -1939,19 +1898,6 @@ class QuicSession extends EventEmitter { state.destroyed = true; state.closing = false; - if (typeof error === 'number' || - (error != null && - typeof error === 'object' && - !(error instanceof Error))) { - const { - closeCode, - closeFamily - } = validateCloseCode(error); - state.closeCode = closeCode; - state.closeFamily = closeFamily; - error = new ERR_QUIC_ERROR(closeCode, closeFamily); - } - // Destroy any pending streams immediately. These // are streams that have been created but have not // yet been assigned an internal handle. @@ -1965,16 +1911,20 @@ class QuicSession extends EventEmitter { this.removeListener('newListener', onNewListener); this.removeListener('removeListener', onRemoveListener); + // If we are destroying with an error, schedule the + // error to be emitted on process.nextTick. if (error) process.nextTick(emit.bind(this, 'error', error)); const handle = this[kHandle]; + this[kHandle] = undefined; + if (handle !== undefined) { // Copy the stats for use after destruction state.stats = new BigInt64Array(handle.stats); - state.idleTimeout = !!handle.state[IDX_QUIC_SESSION_STATE_IDLE_TIMEOUT]; - // Calling destroy will cause a CONNECTION_CLOSE to be - // sent to the peer and will destroy the QuicSession - // handler immediately. + state.idleTimeout = + Boolean(handle.state[IDX_QUIC_SESSION_STATE_IDLE_TIMEOUT]); + + // Destroy the underlying QuicSession handle handle.destroy(state.closeCode, state.closeFamily); } else { process.nextTick(emit.bind(this, 'close')); @@ -2108,7 +2058,8 @@ class QuicSession extends EventEmitter { const state = this[kInternalState]; return { code: state.closeCode, - family: state.closeFamily + family: state.closeFamily, + silent: state.silentClose, }; } diff --git a/src/quic/node_quic_session-inl.h b/src/quic/node_quic_session-inl.h index 339df8210f4a09..73db45ca56deb1 100644 --- a/src/quic/node_quic_session-inl.h +++ b/src/quic/node_quic_session-inl.h @@ -348,7 +348,7 @@ void QuicSession::OnIdleTimeout() { if (!is_destroyed()) { state_[IDX_QUIC_SESSION_STATE_IDLE_TIMEOUT] = 1; Debug(this, "Idle timeout"); - SilentClose(); + Close(QuicSessionListener::SESSION_CLOSE_FLAG_SILENT); } } diff --git a/src/quic/node_quic_session.cc b/src/quic/node_quic_session.cc index 5f102eb2f9362f..8263abc3cc1cab 100644 --- a/src/quic/node_quic_session.cc +++ b/src/quic/node_quic_session.cc @@ -1258,7 +1258,7 @@ bool QuicApplication::SendPendingData() { // to be silent because we can't even send a // CONNECTION_CLOSE since even those require a // packet number. - session()->SilentClose(); + session()->Close(QuicSessionListener::SESSION_CLOSE_FLAG_SILENT); return false; case NGTCP2_ERR_STREAM_DATA_BLOCKED: session()->StreamDataBlocked(stream_data.id); @@ -1629,59 +1629,6 @@ void QuicSession::AddStream(BaseObjectPtr stream) { } } -// Like the silent close, the immediate close must start with -// the JavaScript side, first shutting down any existing -// streams before entering the closing period. Unlike silent -// close, however, all streams are closed using proper -// STOP_SENDING and RESET_STREAM frames and a CONNECTION_CLOSE -// frame is ultimately sent to the peer. This makes the -// naming a bit of a misnomer in that the connection is -// not immediately torn down, but is allowed to drain -// properly per the QUIC spec description of "immediate close". -void QuicSession::ImmediateClose() { - // If ImmediateClose or SilentClose has already been called, - // do not re-enter. - if (is_closing()) - return; - set_closing(); - - QuicError err = last_error(); - Debug(this, "Immediate close with code %" PRIu64 " (%s)", - err.code, - err.family_name()); - - 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) { @@ -1695,32 +1642,80 @@ BaseObjectPtr QuicSession::CreateStream(int64_t stream_id) { return stream; } -// Mark the QuicSession instance destroyed. After this is called, -// the QuicSession instance will be generally unusable but most -// likely will not be immediately freed. -void QuicSession::Destroy() { - if (is_destroyed()) +// Initiate a shutdown of the QuicSession. +void QuicSession::Close(int close_flags) { + CHECK(!is_destroyed()); + bool silent = close_flags & QuicSessionListener::SESSION_CLOSE_FLAG_SILENT; + bool stateless_reset = is_stateless_reset(); + + // If we're not running within a ngtcp2 callback scope, schedule + // a CONNECTION_CLOSE to be sent when Close exits. If we are + // within a ngtcp2 callback scope, sending the CONNECTION_CLOSE + // will be deferred. + ConnectionCloseScope close_scope(this, silent); + + // Once Close has been called, we cannot re-enter + if (UNLIKELY(is_closing())) return; - // If we're not in the closing or draining periods, - // then we should at least attempt to send a connection - // close to the peer. - if (!Ngtcp2CallbackScope::InNgtcp2CallbackScope(this) && - !is_in_closing_period() && - !is_in_draining_period()) { - Debug(this, "Making attempt to send a connection close"); - set_last_error(); - SendConnectionClose(); - } + set_closing(); + set_silent_closing(silent); - // Streams should have already been destroyed by this point. - CHECK(streams_.empty()); + if (stateless_reset && silent) + close_flags |= QuicSessionListener::SESSION_CLOSE_FLAG_STATELESS_RESET; + + QuicError error = last_error(); + Debug(this, "Closing with code %" PRIu64 + " (family: %s, silent: %s, stateless reset: %s)", + error.code, + error.family_name(), + silent ? "Y" : "N", + stateless_reset ? "Y" : "N"); + + // Ensure that the QuicSession is not freed at least until after we + // exit this scope. + BaseObjectPtr ptr(this); + + // If the QuicSession has been wrapped by a JS object, we have to + // notify the JavaScript side that the session is being closed. + // If it hasn't yet been wrapped, we can skip the call and and + // go straight to destroy. + if (is_wrapped()) + listener()->OnSessionClose(error, close_flags); + else + Destroy(); + + // At this point, the QuicSession should have been destroyed, indicating + // that all cleanup on the JavaScript side has completed and the + // QuicSession::Destroy() method has been called. + CHECK(is_destroyed()); +} + +// Mark the QuicSession instance destroyed. This will either be invoked +// synchronously within the callstack of the QuicSession::Close() method +// or not. If it is invoked within QuicSession::Close(), the +// QuicSession::Close() will handle sending the CONNECTION_CLOSE +// frame. +void QuicSession::Destroy() { + if (is_destroyed()) + return; // Mark the session destroyed. set_destroyed(); set_closing(false); set_graceful_closing(false); + // TODO(@jasnell): Allow overriding the close code + + // If we're not already in a ConnectionCloseScope, schedule + // sending a CONNECTION_CLOSE when destroy exits. If we are + // running within an ngtcp2 callback scope, sending the + // CONNECTION_CLOSE will be deferred. + ConnectionCloseScope close_scope(this, is_silent_closing()); + + // All existing streams should have already been destroyed + CHECK(streams_.empty()); + // Stop and free the idle and retransmission timers if they are active. StopIdleTimer(); StopRetransmitTimer(); @@ -1731,7 +1726,8 @@ void QuicSession::Destroy() { // the QuicSession from the QuicSocket will free // that pointer, allowing the QuicSession to be // deconstructed once the stack unwinds and any - // remaining shared_ptr instances fall out of scope. + // remaining BaseObjectPtr instances + // fall out of scope. RemoveFromSocket(); } @@ -1758,7 +1754,7 @@ void QuicSession::HandleError() { if (!SendConnectionClose()) { set_last_error(QUIC_ERROR_SESSION, NGTCP2_ERR_INTERNAL); - ImmediateClose(); + Close(); } } @@ -1933,7 +1929,6 @@ bool QuicSession::Receive( // in the closing period, a CONNECTION_CLOSE has not yet // been sent to the peer. Let's attempt to send one. if (!is_in_closing_period() && !is_in_draining_period()) { - Debug(this, "Attempting to send connection close"); set_last_error(); SendConnectionClose(); } @@ -1949,7 +1944,7 @@ bool QuicSession::Receive( // absolutely nothing left for us to do except silently close // and destroy this QuicSession. GetConnectionCloseInfo(); - SilentClose(); + Close(QuicSessionListener::SESSION_CLOSE_FLAG_SILENT); return true; } Debug(this, "Sending pending data after processing packet"); @@ -1991,7 +1986,7 @@ bool QuicSession::ReceivePacket( // then immediately close the connection. if (err == NGTCP2_ERR_RETRY && is_server()) { socket()->SendRetry(scid_, dcid_, local_address_, remote_address_); - SilentClose(); + Close(QuicSessionListener::SESSION_CLOSE_FLAG_SILENT); break; } set_last_error(QUIC_ERROR_SESSION, err); @@ -2154,35 +2149,17 @@ bool QuicSession::SendConnectionClose() { return true; } + UpdateIdleTimer(); switch (crypto_context_->side()) { case NGTCP2_CRYPTO_SIDE_SERVER: { - // If we're not already in the closing period, - // first attempt to write any pending packets, then - // start the closing period. If closing period has - // already started, skip this. - if (!is_in_closing_period() && - (!WritePackets("server connection close - write packets") || - !StartClosingPeriod())) { - return false; - } - - UpdateIdleTimer(); + if (!is_in_closing_period() && !StartClosingPeriod()) + return false; CHECK_GT(conn_closebuf_->length(), 0); - return SendPacket(QuicPacket::Copy(conn_closebuf_)); } case NGTCP2_CRYPTO_SIDE_CLIENT: { - UpdateIdleTimer(); auto packet = QuicPacket::Create("client connection close"); - // If we're not already in the closing period, - // first attempt to write any pending packets, then - // start the closing period. Note that the behavior - // here is different than the server - if (!is_in_closing_period() && - !WritePackets("client connection close - write packets")) { - return false; - } ssize_t nwrite = SelectCloseFn(error.family)( connection(), @@ -2191,7 +2168,7 @@ bool QuicSession::SendConnectionClose() { max_pktlen_, error.code, uv_hrtime()); - if (nwrite < 0) { + if (UNLIKELY(nwrite < 0)) { Debug(this, "Error writing connection close: %d", nwrite); set_last_error(QUIC_ERROR_SESSION, static_cast(nwrite)); return false; @@ -2233,7 +2210,6 @@ void QuicSession::UsePreferredAddressStrategy( // Passes a serialized packet to the associated QuicSocket. bool QuicSession::SendPacket(std::unique_ptr packet) { - CHECK(!is_destroyed()); CHECK(!is_in_draining_period()); // There's nothing to send. @@ -2391,7 +2367,7 @@ bool QuicSession::StartClosingPeriod() { if (nwrite < 0) { if (nwrite == NGTCP2_ERR_PKT_NUM_EXHAUSTED) { set_last_error(QUIC_ERROR_SESSION, NGTCP2_ERR_PKT_NUM_EXHAUSTED); - SilentClose(); + Close(QuicSessionListener::SESSION_CLOSE_FLAG_SILENT); } else { set_last_error(QUIC_ERROR_SESSION, static_cast(nwrite)); } @@ -2516,16 +2492,11 @@ void QuicSession::UpdateIdleTimer() { bool QuicSession::WritePackets(const char* diagnostic_label) { CHECK(!Ngtcp2CallbackScope::InNgtcp2CallbackScope(this)); - // During the draining period, we must not send any frames at all. - if (is_in_draining_period()) + // During either the draining or closing period, + // we are not permitted to send any additional packets. + if (is_in_draining_period() || is_in_closing_period()) return true; - // During the closing period, we are only permitted to send - // CONNECTION_CLOSE frames. - if (is_in_closing_period()) { - return SendConnectionClose(); - } - // Otherwise, serialize and send pending frames QuicPathStorage path; for (;;) { @@ -2541,9 +2512,11 @@ bool QuicSession::WritePackets(const char* diagnostic_label) { packet->data(), max_pktlen_, uv_hrtime()); + if (nwrite <= 0) { switch (nwrite) { case 0: + // There was nothing to write. return true; case NGTCP2_ERR_PKT_NUM_EXHAUSTED: // There is a finite number of packets that can be sent @@ -2553,7 +2526,7 @@ bool QuicSession::WritePackets(const char* diagnostic_label) { // to be silent because we can't even send a // CONNECTION_CLOSE since even those require a // packet number. - SilentClose(); + Close(QuicSessionListener::SESSION_CLOSE_FLAG_SILENT); return false; default: set_last_error(QUIC_ERROR_SESSION, static_cast(nwrite)); @@ -3359,23 +3332,6 @@ void QuicSessionSetSocket(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(session->set_socket(socket)); } -// Perform an immediate close on the QuicSession, causing a -// CONNECTION_CLOSE frame to be scheduled and sent and starting -// the closing period for this session. The name "ImmediateClose" -// is a bit of an unfortunate misnomer as the session will not -// be immediately shutdown. The naming is pulled from the QUIC -// spec to indicate a state where the session immediately enters -// the closing period, but the session will not be destroyed -// until either the idle timeout fires or destroy is explicitly -// called. -void QuicSessionClose(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - QuicSession* session; - ASSIGN_OR_RETURN_UNWRAP(&session, args.Holder()); - session->set_last_error(QuicError(env, args[0], args[1])); - session->SendConnectionClose(); -} - // GracefulClose flips a flag that prevents new local streams // from being opened and new remote streams from being received. It is // important to note that this does *NOT* send a CONNECTION_CLOSE packet @@ -3445,7 +3401,7 @@ void QuicSessionSilentClose(const FunctionCallbackInfo& args) { ProcessEmitWarning( session->env(), "Forcing silent close of QuicSession for testing purposes only"); - session->SilentClose(); + session->Close(QuicSessionListener::SESSION_CLOSE_FLAG_SILENT); } // TODO(addaleax): This is a temporary solution for testing and should be @@ -3592,7 +3548,6 @@ void NewQuicClientSession(const FunctionCallbackInfo& args) { // Add methods that are shared by both client and server QuicSessions void AddMethods(Environment* env, Local session) { - env->SetProtoMethod(session, "close", QuicSessionClose); env->SetProtoMethod(session, "destroy", QuicSessionDestroy); env->SetProtoMethod(session, "getRemoteAddress", QuicSessionGetRemoteAddress); env->SetProtoMethod(session, "getCertificate", QuicSessionGetCertificate); diff --git a/src/quic/node_quic_session.h b/src/quic/node_quic_session.h index 3087ecf12c947b..c68291aed67858 100644 --- a/src/quic/node_quic_session.h +++ b/src/quic/node_quic_session.h @@ -691,11 +691,13 @@ class QuicApplication : public MemoryRetainer, // QUICSESSION_FLAGS are converted into is_{name}() and set_{name}(bool on) // accessors on the QuicSession class. #define QUICSESSION_FLAGS(V) \ + V(WRAPPED, wrapped) \ V(CLOSING, closing) \ V(GRACEFUL_CLOSING, graceful_closing) \ V(DESTROYED, destroyed) \ V(TRANSPORT_PARAMS_SET, transport_params_set) \ V(NGTCP2_CALLBACK, in_ngtcp2_callback) \ + V(CONNECTION_CLOSE_SCOPE, in_connection_close_scope) \ V(SILENT_CLOSE, silent_closing) \ V(STATELESS_RESET, stateless_reset) @@ -847,7 +849,7 @@ class QuicSession : public AsyncWrap, #undef V // Returns true if the QuicSession has entered the - // closing period following a call to ImmediateClose. + // closing period after sending a CONNECTION_CLOSE. // While true, the QuicSession is only permitted to // transmit CONNECTION_CLOSE frames until either the // idle timeout period elapses or until the QuicSession @@ -871,7 +873,7 @@ class QuicSession : public AsyncWrap, // be immediately closed once there are no remaining streams. Note // that no notification is given to the connecting peer that we're // in a graceful closing state. A CONNECTION_CLOSE will be sent only - // once ImmediateClose() is called. + // once Close() is called. inline void StartGracefulClose(); QuicError last_error() const { return last_error_; } @@ -1040,17 +1042,16 @@ class QuicSession : public AsyncWrap, inline void DecreaseAllocatedSize(size_t size); - // 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 - // typically only done during an idle timeout or when sending - // a retry packet. - void SilentClose(); + // Initiate closing of the QuicSession. This will round trip + // through JavaScript, causing all currently opened streams + // to be closed. If the SESSION_CLOSE_FLAG_SILENT flag is + // set, the connected peer will not be notified, otherwise + // an attempt will be made to send a CONNECTION_CLOSE frame + // to the peer. If Close is called while within the ngtcp2 + // callback scope, sending the CONNECTION_CLOSE will be + // deferred until the ngtcp2 callback scope exits. + inline void Close( + int close_flags = QuicSessionListener::SESSION_CLOSE_FLAG_NONE); void PushListener(QuicSessionListener* listener); @@ -1087,12 +1088,48 @@ class QuicSession : public AsyncWrap, } ~SendSessionScope() { - if (!Ngtcp2CallbackScope::InNgtcp2CallbackScope(session_.get())) - session_->SendPendingData(); + if (Ngtcp2CallbackScope::InNgtcp2CallbackScope(session_.get()) || + session_->is_in_closing_period() || + session_->is_in_draining_period()) { + return; + } + session_->SendPendingData(); + } + + private: + BaseObjectPtr session_; + }; + + // ConnectionCloseScope triggers sending a CONNECTION_CLOSE + // when not executing within the context of an ngtcp2 callback + // and the session is in the correct state. + class ConnectionCloseScope { + public: + ConnectionCloseScope(QuicSession* session, bool silent = false) + : session_(session), + silent_(silent) { + CHECK(session_); + // If we are already in a ConnectionCloseScope, ignore. + if (session_->is_in_connection_close_scope()) + silent_ = true; + else + session_->set_in_connection_close_scope(); + } + + ~ConnectionCloseScope() { + if (silent_ || + Ngtcp2CallbackScope::InNgtcp2CallbackScope(session_.get()) || + session_->is_in_closing_period() || + session_->is_in_draining_period()) { + return; + } + session_->set_in_connection_close_scope(false); + bool ret = session_->SendConnectionClose(); } private: BaseObjectPtr session_; + bool silent_ = false; }; // Tracks whether or not we are currently within an ngtcp2 callback diff --git a/src/quic/node_quic_socket.cc b/src/quic/node_quic_socket.cc index 6ebdb30ef8a250..b51edee6c9be9a 100644 --- a/src/quic/node_quic_socket.cc +++ b/src/quic/node_quic_socket.cc @@ -843,6 +843,8 @@ BaseObjectPtr QuicSocket::AcceptInitialPacket( local_addr, remote_addr, NGTCP2_CONNECTION_REFUSED); + } else { + session->set_wrapped(); } return session; From b78df93d8805043677e4992637faaf5df3ab6114 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Wed, 1 Jul 2020 16:40:16 -0700 Subject: [PATCH 05/11] quic: refactor QuicSession shared state to use AliasedStruct --- lib/internal/quic/core.js | 49 ++++++------ lib/internal/quic/util.js | 133 ++++++++++++++++++++++++++++--- src/quic/node_quic.cc | 16 ++-- src/quic/node_quic_session-inl.h | 14 ++-- src/quic/node_quic_session.cc | 29 +++---- src/quic/node_quic_session.h | 96 +++++++--------------- 6 files changed, 196 insertions(+), 141 deletions(-) diff --git a/lib/internal/quic/core.js b/lib/internal/quic/core.js index 75f07c8d7e5871..00907398b09b8d 100644 --- a/lib/internal/quic/core.js +++ b/lib/internal/quic/core.js @@ -39,6 +39,7 @@ const { validateQuicEndpointOptions, validateCreateSecureContextOptions, validateQuicSocketConnectOptions, + QuicSessionSharedState, } = require('internal/quic/util'); const util = require('util'); const assert = require('internal/assert'); @@ -131,12 +132,6 @@ const { AF_INET, AF_INET6, NGTCP2_DEFAULT_MAX_PKTLEN, - IDX_QUIC_SESSION_STATE_MAX_STREAMS_BIDI, - IDX_QUIC_SESSION_STATE_MAX_STREAMS_UNI, - IDX_QUIC_SESSION_STATE_MAX_DATA_LEFT, - IDX_QUIC_SESSION_STATE_HANDSHAKE_CONFIRMED, - IDX_QUIC_SESSION_STATE_IDLE_TIMEOUT, - IDX_QUIC_SESSION_STATE_BYTES_IN_FLIGHT, IDX_QUIC_SESSION_STATS_CREATED_AT, IDX_QUIC_SESSION_STATS_HANDSHAKE_START_AT, IDX_QUIC_SESSION_STATS_BYTES_RECEIVED, @@ -605,11 +600,11 @@ function createSecureContext(options, init_cb) { } function onNewListener(event) { - toggleListeners(this[kHandle], event, true); + toggleListeners(this[kInternalState].state, event, true); } function onRemoveListener(event) { - toggleListeners(this[kHandle], event, false); + toggleListeners(this[kInternalState].state, event, false); } function getStats(obj, idx) { @@ -1651,6 +1646,7 @@ class QuicSession extends EventEmitter { handshakeContinuationHistogram: undefined, highWaterMark: undefined, defaultEncoding: undefined, + state: undefined, }; constructor(socket, options) { @@ -1693,6 +1689,7 @@ class QuicSession extends EventEmitter { this[kHandle] = handle; if (handle !== undefined) { handle[owner_symbol] = this; + state.state = new QuicSessionSharedState(handle.state); state.handshakeAckHistogram = new Histogram(handle.ack); state.handshakeContinuationHistogram = new Histogram(handle.rate); } else { @@ -1849,10 +1846,10 @@ class QuicSession extends EventEmitter { return false; } - // Closing allows any existing QuicStream's to complete - // normally but disallows any new QuicStreams from being - // opened. Calls to openStream() will fail, and new streams - // from the peer will be rejected/ignored. + // Closing allows any existing QuicStream's to gracefully + // complete while disallowing any new QuicStreams from being + // opened (in either direction). Calls to openStream() will + // fail, and new streams from the peer will be rejected/ignored. close(callback) { const state = this[kInternalState]; if (state.destroyed) @@ -1921,8 +1918,7 @@ class QuicSession extends EventEmitter { if (handle !== undefined) { // Copy the stats for use after destruction state.stats = new BigInt64Array(handle.stats); - state.idleTimeout = - Boolean(handle.state[IDX_QUIC_SESSION_STATE_IDLE_TIMEOUT]); + state.idleTimeout = this[kInternalState].state.idleTimeout; // Destroy the underlying QuicSession handle handle.destroy(state.closeCode, state.closeFamily); @@ -1950,8 +1946,8 @@ class QuicSession extends EventEmitter { let bidi = 0; let uni = 0; if (this[kHandle]) { - bidi = this[kHandle].state[IDX_QUIC_SESSION_STATE_MAX_STREAMS_BIDI]; - uni = this[kHandle].state[IDX_QUIC_SESSION_STATE_MAX_STREAMS_UNI]; + bidi = this[kInternalState].state.maxStreamsBidi; + uni = this[kInternalState].state.maxStreamsUni; } return { bidi, uni }; } @@ -1961,15 +1957,15 @@ class QuicSession extends EventEmitter { } get maxDataLeft() { - return this[kHandle]?.state[IDX_QUIC_SESSION_STATE_MAX_DATA_LEFT] || 0; + return this[kHandle] ? this[kInternalState].state.maxDataLeft : 0; } get bytesInFlight() { - return this[kHandle]?.state[IDX_QUIC_SESSION_STATE_BYTES_IN_FLIGHT] || 0; + return this[kHandle] ? this[kInternalState].state.bytesInFlight : 0; } get blockCount() { - return this[kHandle]?.state[IDX_QUIC_SESSION_STATS_BLOCK_COUNT] || 0; + return this[kHandle]?.stats[IDX_QUIC_SESSION_STATS_BLOCK_COUNT] || 0; } get authenticated() { @@ -2003,8 +1999,9 @@ class QuicSession extends EventEmitter { } get handshakeConfirmed() { - return Boolean( - this[kHandle]?.state[IDX_QUIC_SESSION_STATE_HANDSHAKE_CONFIRMED]); + return this[kHandle] ? + this[kInternalState].state.handshakeConfirmed : + false; } get idleTimeout() { @@ -2449,14 +2446,16 @@ class QuicClientSession extends QuicSession { // Listeners may have been added before the handle was created. // Ensure that we toggle those listeners in the handle state. - if (this.listenerCount('keylog') > 0) - toggleListeners(handle, 'keylog', true); + const internalState = this[kInternalState]; + if (this.listenerCount('keylog') > 0) { + toggleListeners(internalState.state, 'keylog', true); + } if (this.listenerCount('pathValidation') > 0) - toggleListeners(handle, 'pathValidation', true); + toggleListeners(internalState.state, 'pathValidation', true); if (this.listenerCount('usePreferredAddress') > 0) - toggleListeners(handle, 'usePreferredAddress', true); + toggleListeners(internalState.state, 'usePreferredAddress', true); this[kMaybeReady](0x2); } diff --git a/lib/internal/quic/util.js b/lib/internal/quic/util.js index efeddee94b6e64..6bbe10959723a9 100644 --- a/lib/internal/quic/util.js +++ b/lib/internal/quic/util.js @@ -15,6 +15,12 @@ const { }, } = require('internal/errors'); +const { + kHandle, +} = require('internal/stream_base_commons'); + +const endianness = require('os').endianness(); + const assert = require('internal/assert'); assert(process.versions.ngtcp2 !== undefined); @@ -52,11 +58,19 @@ const { IDX_QUIC_SESSION_MAX_UDP_PAYLOAD_SIZE, IDX_QUIC_SESSION_CC_ALGO, IDX_QUIC_SESSION_CONFIG_COUNT, - IDX_QUIC_SESSION_STATE_CERT_ENABLED, - IDX_QUIC_SESSION_STATE_CLIENT_HELLO_ENABLED, - IDX_QUIC_SESSION_STATE_KEYLOG_ENABLED, - IDX_QUIC_SESSION_STATE_PATH_VALIDATED_ENABLED, - IDX_QUIC_SESSION_STATE_USE_PREFERRED_ADDRESS_ENABLED, + + IDX_QUICSESSION_STATE_KEYLOG_ENABLED, + IDX_QUICSESSION_STATE_CLIENT_HELLO_ENABLED, + IDX_QUICSESSION_STATE_CERT_ENABLED, + IDX_QUICSESSION_STATE_PATH_VALIDATED_ENABLED, + IDX_QUICSESSION_STATE_USE_PREFERRED_ADDRESS_ENABLED, + IDX_QUICSESSION_STATE_HANDSHAKE_CONFIRMED, + IDX_QUICSESSION_STATE_IDLE_TIMEOUT, + IDX_QUICSESSION_STATE_MAX_STREAMS_BIDI, + IDX_QUICSESSION_STATE_MAX_STREAMS_UNI, + IDX_QUICSESSION_STATE_MAX_DATA_LEFT, + IDX_QUICSESSION_STATE_BYTES_IN_FLIGHT, + IDX_HTTP3_QPACK_MAX_TABLE_CAPACITY, IDX_HTTP3_QPACK_BLOCKED_STREAMS, IDX_HTTP3_MAX_HEADER_LIST_SIZE, @@ -756,29 +770,121 @@ function setTransportParams(config) { // communicate that a handler has been added for the optional events // so that the C++ internals know there is an actual listener. The event // will not be emitted if there is no handler. -function toggleListeners(handle, event, on) { - if (handle === undefined) +function toggleListeners(state, event, on) { + if (state === undefined) return; - const val = on ? 1 : 0; switch (event) { case 'keylog': - handle.state[IDX_QUIC_SESSION_STATE_KEYLOG_ENABLED] = val; + state.keylogEnabled = on; break; case 'clientHello': - handle.state[IDX_QUIC_SESSION_STATE_CLIENT_HELLO_ENABLED] = val; + state.clientHelloEnabled = on; break; case 'pathValidation': - handle.state[IDX_QUIC_SESSION_STATE_PATH_VALIDATED_ENABLED] = val; + state.pathValidatedEnabled = on; break; case 'OCSPRequest': - handle.state[IDX_QUIC_SESSION_STATE_CERT_ENABLED] = val; + state.certEnabled = on; break; case 'usePreferredAddress': - handle.state[IDX_QUIC_SESSION_STATE_USE_PREFERRED_ADDRESS_ENABLED] = on; + state.usePreferredAddressEnabled = on; break; } } +// A utility class used to handle reading / modifying shared JS/C++ +// state associated with a QuicSession +class QuicSessionSharedState { + constructor(state) { + this[kHandle] = Buffer.from(state); + } + + get keylogEnabled() { + return Boolean(this[kHandle] + .readUInt8(IDX_QUICSESSION_STATE_KEYLOG_ENABLED)); + } + + set keylogEnabled(on) { + this[kHandle] + .writeUInt8(on ? 1 : 0, IDX_QUICSESSION_STATE_KEYLOG_ENABLED); + } + + get clientHelloEnabled() { + return Boolean(this[kHandle] + .readUInt8(IDX_QUICSESSION_STATE_CLIENT_HELLO_ENABLED)); + } + + set clientHelloEnabled(on) { + this[kHandle] + .writeUInt8(on ? 1 : 0, IDX_QUICSESSION_STATE_CLIENT_HELLO_ENABLED); + } + + get certEnabled() { + return Boolean(this[kHandle] + .readUInt8(IDX_QUICSESSION_STATE_CERT_ENABLED)); + } + + set certEnabled(on) { + this[kHandle] + .writeUInt8(on ? 1 : 0, IDX_QUICSESSION_STATE_CERT_ENABLED); + } + + get pathValidatedEnabled() { + return Boolean(this[kHandle] + .readUInt8(IDX_QUICSESSION_STATE_PATH_VALIDATED_ENABLED)); + } + + set pathValidatedEnabled(on) { + this[kHandle] + .writeUInt8(on ? 1 : 0, IDX_QUICSESSION_STATE_PATH_VALIDATED_ENABLED); + } + + get usePreferredAddressEnabled() { + return Boolean(this[kHandle] + .readUInt8(IDX_QUICSESSION_STATE_USE_PREFERRED_ADDRESS_ENABLED)); + } + + set usePreferredAddressEnabled(on) { + this[kHandle] + .writeUInt8(on ? 1 : 0, + IDX_QUICSESSION_STATE_USE_PREFERRED_ADDRESS_ENABLED); + } + + get handshakeConfirmed() { + return Boolean(this[kHandle] + .readUInt8(IDX_QUICSESSION_STATE_HANDSHAKE_CONFIRMED)); + } + + get idleTimeout() { + return Boolean(this[kHandle] + .readUInt8(IDX_QUICSESSION_STATE_IDLE_TIMEOUT)); + } + + get maxStreamsBidi() { + return Number(endianness === 'BE' ? + this[kHandle].readBigInt64BE(IDX_QUICSESSION_STATE_MAX_STREAMS_BIDI) : + this[kHandle].readBigInt64LE(IDX_QUICSESSION_STATE_MAX_STREAMS_BIDI)); + } + + get maxStreamsUni() { + return Number(endianness === 'BE' ? + this[kHandle].readBigInt64BE(IDX_QUICSESSION_STATE_MAX_STREAMS_UNI) : + this[kHandle].readBigInt64LE(IDX_QUICSESSION_STATE_MAX_STREAMS_UNI)); + } + + get maxDataLeft() { + return Number(endianness === 'BE' ? + this[kHandle].readBigInt64BE(IDX_QUICSESSION_STATE_MAX_DATA_LEFT) : + this[kHandle].readBigInt64LE(IDX_QUICSESSION_STATE_MAX_DATA_LEFT)); + } + + get bytesInFlight() { + return Number(endianness === 'BE' ? + this[kHandle].readBigInt64BE(IDX_QUICSESSION_STATE_BYTES_IN_FLIGHT) : + this[kHandle].readBigInt64LE(IDX_QUICSESSION_STATE_BYTES_IN_FLIGHT)); + } +} + module.exports = { getAllowUnauthorized, getSocketType, @@ -796,4 +902,5 @@ module.exports = { validateQuicEndpointOptions, validateCreateSecureContextOptions, validateQuicSocketConnectOptions, + QuicSessionSharedState, }; diff --git a/src/quic/node_quic.cc b/src/quic/node_quic.cc index ed147eb92b649f..49aba2e5db2870 100644 --- a/src/quic/node_quic.cc +++ b/src/quic/node_quic.cc @@ -173,17 +173,6 @@ void Initialize(Local target, V(IDX_QUIC_SESSION_MAX_ACK_DELAY) \ V(IDX_QUIC_SESSION_CC_ALGO) \ V(IDX_QUIC_SESSION_CONFIG_COUNT) \ - V(IDX_QUIC_SESSION_STATE_CERT_ENABLED) \ - V(IDX_QUIC_SESSION_STATE_CLIENT_HELLO_ENABLED) \ - V(IDX_QUIC_SESSION_STATE_USE_PREFERRED_ADDRESS_ENABLED) \ - V(IDX_QUIC_SESSION_STATE_PATH_VALIDATED_ENABLED) \ - V(IDX_QUIC_SESSION_STATE_KEYLOG_ENABLED) \ - V(IDX_QUIC_SESSION_STATE_MAX_STREAMS_BIDI) \ - V(IDX_QUIC_SESSION_STATE_MAX_STREAMS_UNI) \ - V(IDX_QUIC_SESSION_STATE_MAX_DATA_LEFT) \ - V(IDX_QUIC_SESSION_STATE_BYTES_IN_FLIGHT) \ - V(IDX_QUIC_SESSION_STATE_HANDSHAKE_CONFIRMED) \ - V(IDX_QUIC_SESSION_STATE_IDLE_TIMEOUT) \ V(MAX_RETRYTOKEN_EXPIRATION) \ V(MIN_RETRYTOKEN_EXPIRATION) \ V(NGTCP2_APP_NOERROR) \ @@ -212,6 +201,11 @@ void Initialize(Local target, V(ERR_FAILED_TO_CREATE_SESSION) \ V(UV_EBADF) +#define V(id, _, __) \ + NODE_DEFINE_CONSTANT(constants, IDX_QUICSESSION_STATE_##id); + QUICSESSION_SHARED_STATE(V) +#undef V + #define V(name, _, __) \ NODE_DEFINE_CONSTANT(constants, IDX_QUIC_SESSION_STATS_##name); SESSION_STATS(V) diff --git a/src/quic/node_quic_session-inl.h b/src/quic/node_quic_session-inl.h index 73db45ca56deb1..935c8d024d55c9 100644 --- a/src/quic/node_quic_session-inl.h +++ b/src/quic/node_quic_session-inl.h @@ -105,7 +105,7 @@ ngtcp2_crypto_level QuicCryptoContext::write_crypto_level() const { // to a keylog file that can be consumed by tools like Wireshark to intercept // and decrypt QUIC network traffic. void QuicCryptoContext::Keylog(const char* line) { - if (UNLIKELY(session_->state_[IDX_QUIC_SESSION_STATE_KEYLOG_ENABLED] == 1)) + if (UNLIKELY(session_->state_->keylog_enabled)) session_->listener()->OnKeylog(line, strlen(line)); } @@ -117,7 +117,7 @@ void QuicCryptoContext::OnClientHelloDone() { [&]() { set_in_client_hello(false); }); // Disable the callback at this point so we don't loop continuously - session_->state_[IDX_QUIC_SESSION_STATE_CLIENT_HELLO_ENABLED] = 0; + session_->state_->client_hello_enabled = 0; } // Following a pause in the handshake for OCSP or client hello, we kickstart @@ -274,14 +274,12 @@ void QuicSession::ExtendMaxStreamsRemoteBidi(uint64_t max_streams) { void QuicSession::ExtendMaxStreamsUni(uint64_t max_streams) { Debug(this, "Setting max unidirectional streams to %" PRIu64, max_streams); - state_[IDX_QUIC_SESSION_STATE_MAX_STREAMS_UNI] = - static_cast(max_streams); + state_->max_streams_uni = max_streams; } void QuicSession::ExtendMaxStreamsBidi(uint64_t max_streams) { Debug(this, "Setting max bidirectional streams to %" PRIu64, max_streams); - state_[IDX_QUIC_SESSION_STATE_MAX_STREAMS_BIDI] = - static_cast(max_streams); + state_->max_streams_bidi = max_streams; } // Extends the stream-level flow control by the given number of bytes. @@ -327,7 +325,7 @@ void QuicSession::HandshakeCompleted() { void QuicSession::HandshakeConfirmed() { Debug(this, "Handshake is confirmed"); RecordTimestamp(&QuicSessionStats::handshake_confirmed_at); - state_[IDX_QUIC_SESSION_STATE_HANDSHAKE_CONFIRMED] = 1; + state_->handshake_confirmed = 1; } bool QuicSession::is_handshake_completed() const { @@ -346,7 +344,7 @@ void QuicSession::InitApplication() { // the peer. All existing streams are abandoned and closed. void QuicSession::OnIdleTimeout() { if (!is_destroyed()) { - state_[IDX_QUIC_SESSION_STATE_IDLE_TIMEOUT] = 1; + state_->idle_timeout = 1; Debug(this, "Idle timeout"); Close(QuicSessionListener::SESSION_CLOSE_FLAG_SILENT); } diff --git a/src/quic/node_quic_session.cc b/src/quic/node_quic_session.cc index 8263abc3cc1cab..ffc4f25bfcc9d8 100644 --- a/src/quic/node_quic_session.cc +++ b/src/quic/node_quic_session.cc @@ -1,5 +1,6 @@ #include "node_quic_session-inl.h" // NOLINT(build/include) #include "aliased_buffer.h" +#include "aliased_struct-inl.h" #include "allocated_buffer-inl.h" #include "debug_utils-inl.h" #include "env-inl.h" @@ -919,10 +920,8 @@ void QuicCryptoContext::EnableTrace() { // when the peer certificate is received, allowing additional tweaks // and verifications to be performed. int QuicCryptoContext::OnClientHello() { - if (LIKELY(session_->state_[ - IDX_QUIC_SESSION_STATE_CLIENT_HELLO_ENABLED] == 0)) { + if (LIKELY(session_->state_->client_hello_enabled == 0)) return 0; - } TLSCallbackScope callback_scope(this); @@ -952,7 +951,7 @@ int QuicCryptoContext::OnClientHello() { // function that must be called in order for the TLS handshake to // continue. int QuicCryptoContext::OnOCSP() { - if (LIKELY(session_->state_[IDX_QUIC_SESSION_STATE_CERT_ENABLED] == 0)) { + if (LIKELY(session_->state_->cert_enabled == 0)) { Debug(session(), "No OCSPRequest handler registered"); return 1; } @@ -991,7 +990,7 @@ void QuicCryptoContext::OnOCSPDone( [&]() { set_in_ocsp_request(false); }); // Disable the callback at this point so we don't loop continuously - session_->state_[IDX_QUIC_SESSION_STATE_CERT_ENABLED] = 0; + session_->state_->cert_enabled = 0; if (context) { int err = crypto::UseSNIContext(ssl_, context); @@ -1446,7 +1445,7 @@ QuicSession::QuicSession( idle_(new Timer(socket->env(), [this]() { OnIdleTimeout(); })), retransmit_(new Timer(socket->env(), [this]() { MaybeTimeout(); })), dcid_(dcid), - state_(env()->isolate(), IDX_QUIC_SESSION_STATE_COUNT), + state_(env()->isolate()), quic_state_(socket->quic_state()) { PushListener(&default_listener_); set_connection_id_strategy(RandomConnectionIDStrategy); @@ -1459,13 +1458,10 @@ QuicSession::QuicSession( options)); application_.reset(SelectApplication(this)); - // TODO(@jasnell): For now, the following is a check rather than properly - // handled. Before this code moves out of experimental, this should be - // properly handled. wrap->DefineOwnProperty( env()->context(), env()->state_string(), - state_.GetJSArray(), + state_.GetArrayBuffer(), PropertyAttribute::ReadOnly).Check(); // TODO(@jasnell): memory accounting @@ -1814,7 +1810,7 @@ void QuicSession::PathValidation( // Only emit the callback if there is a handler for the pathValidation // event on the JavaScript QuicSession object. - if (UNLIKELY(state_[IDX_QUIC_SESSION_STATE_PATH_VALIDATED_ENABLED] == 1)) { + if (UNLIKELY(state_->path_validated_enabled == 1)) { listener_->OnPathValidation( res, reinterpret_cast(path->local.addr), @@ -2190,14 +2186,12 @@ void QuicSession::IgnorePreferredAddressStrategy( void QuicSession::UsePreferredAddressStrategy( QuicSession* session, const PreferredAddress& preferred_address) { - static constexpr int idx = - IDX_QUIC_SESSION_STATE_USE_PREFERRED_ADDRESS_ENABLED; int family = session->socket()->local_address().family(); if (preferred_address.Use(family)) { Debug(session, "Using server preferred address"); // Emit only if the QuicSession has a usePreferredAddress handler // on the JavaScript side. - if (UNLIKELY(session->state_[idx] == 1)) { + if (UNLIKELY(session->state_->use_preferred_address_enabled == 1)) { session->listener()->OnUsePreferredAddress(family, preferred_address); } } else { @@ -2550,7 +2544,6 @@ void QuicSession::MemoryInfo(MemoryTracker* tracker) const { tracker->TrackField("idle", idle_); tracker->TrackField("retransmit", retransmit_); tracker->TrackField("streams", streams_); - tracker->TrackField("state", state_); tracker->TrackFieldWithSize("current_ngtcp2_memory", current_ngtcp2_memory_); tracker->TrackField("conn_closebuf", conn_closebuf_); tracker->TrackField("application", application_); @@ -2697,14 +2690,12 @@ void QuicSession::UpdateRecoveryStats() { void QuicSession::UpdateDataStats() { if (is_destroyed()) return; - state_[IDX_QUIC_SESSION_STATE_MAX_DATA_LEFT] = - static_cast(ngtcp2_conn_get_max_data_left(connection())); + state_->max_data_left = ngtcp2_conn_get_max_data_left(connection()); ngtcp2_conn_stat stat; ngtcp2_conn_get_conn_stat(connection(), &stat); - state_[IDX_QUIC_SESSION_STATE_BYTES_IN_FLIGHT] = - static_cast(stat.bytes_in_flight); + state_->bytes_in_flight = stat.bytes_in_flight; // The max_bytes_in_flight is a highwater mark that can be used // in performance analysis operations. if (stat.bytes_in_flight > GetStat(&QuicSessionStats::max_bytes_in_flight)) diff --git a/src/quic/node_quic_session.h b/src/quic/node_quic_session.h index c68291aed67858..acf433622b38d3 100644 --- a/src/quic/node_quic_session.h +++ b/src/quic/node_quic_session.h @@ -4,6 +4,7 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS #include "aliased_buffer.h" +#include "aliased_struct.h" #include "async_wrap.h" #include "env.h" #include "handle_wrap.h" @@ -141,67 +142,32 @@ enum QuicClientSessionOptions : uint32_t { QUICCLIENTSESSION_OPTION_RESUME = 0x4 }; +#define QUICSESSION_SHARED_STATE(V) \ + V(KEYLOG_ENABLED, keylog_enabled, uint8_t) \ + V(CLIENT_HELLO_ENABLED, client_hello_enabled, uint8_t) \ + V(CERT_ENABLED, cert_enabled, uint8_t) \ + V(PATH_VALIDATED_ENABLED, path_validated_enabled, uint8_t) \ + V(USE_PREFERRED_ADDRESS_ENABLED, use_preferred_address_enabled, uint8_t) \ + V(HANDSHAKE_CONFIRMED, handshake_confirmed, uint8_t) \ + V(IDLE_TIMEOUT, idle_timeout, uint8_t) \ + V(MAX_STREAMS_BIDI, max_streams_bidi, uint64_t) \ + V(MAX_STREAMS_UNI, max_streams_uni, uint64_t) \ + V(MAX_DATA_LEFT, max_data_left, uint64_t) \ + V(BYTES_IN_FLIGHT, bytes_in_flight, uint64_t) + +#define V(_, name, type) type name; +struct QuicSessionState { + QUICSESSION_SHARED_STATE(V) +}; +#undef V -// The QuicSessionState enums are used with the QuicSession's -// private state_ array. This is exposed to JavaScript via an -// aliased buffer and is used to communicate various types of -// state efficiently across the native/JS boundary. -enum QuicSessionState : int { - // Communicates whether a 'keylog' event listener has been - // registered on the JavaScript QuicSession object. The - // value will be either 1 or 0. When set to 1, the native - // code will emit TLS keylog entries to the JavaScript - // side triggering the 'keylog' event once for each line. - IDX_QUIC_SESSION_STATE_KEYLOG_ENABLED, - - // Communicates whether a 'clientHello' event listener has - // been registered on the JavaScript QuicServerSession. - // The value will be either 1 or 0. When set to 1, the - // native code will callout to the JavaScript side causing - // the 'clientHello' event to be emitted. This is only - // used on server QuicSession instances. - IDX_QUIC_SESSION_STATE_CLIENT_HELLO_ENABLED, - - // Communicates whether a 'cert' event listener has been - // registered on the JavaScript QuicSession. The value will - // be either 1 or 0. When set to 1, the native code will - // callout to the JavaScript side causing the 'cert' event - // to be emitted. - IDX_QUIC_SESSION_STATE_CERT_ENABLED, - - // Communicates whether a 'pathValidation' event listener - // has been registered on the JavaScript QuicSession. The - // value will be either 1 or 0. When set to 1, the native - // code will callout to the JavaScript side causing the - // 'pathValidation' event to be emitted - IDX_QUIC_SESSION_STATE_PATH_VALIDATED_ENABLED, - - // Communicates the current max cumulative number of - // bidi and uni streams that may be opened on the session - IDX_QUIC_SESSION_STATE_MAX_STREAMS_BIDI, - IDX_QUIC_SESSION_STATE_MAX_STREAMS_UNI, - - // Communicates the current maxinum number of bytes that - // the local endpoint can send in this connection - // (updated immediately after processing sent/received packets) - IDX_QUIC_SESSION_STATE_MAX_DATA_LEFT, - - // Communicates the current total number of bytes in flight - IDX_QUIC_SESSION_STATE_BYTES_IN_FLIGHT, - - // Communicates whether a 'usePreferredAddress' event listener - // has been registered. - IDX_QUIC_SESSION_STATE_USE_PREFERRED_ADDRESS_ENABLED, - - IDX_QUIC_SESSION_STATE_HANDSHAKE_CONFIRMED, - - // Communicates whether a session was closed due to idle timeout - IDX_QUIC_SESSION_STATE_IDLE_TIMEOUT, - - // Just the number of session state enums for use when - // creating the AliasedBuffer. - IDX_QUIC_SESSION_STATE_COUNT +#define V(id, name, _) \ + IDX_QUICSESSION_STATE_##id = offsetof(QuicSessionState, name), +enum QuicSessionStateFields { + QUICSESSION_SHARED_STATE(V) + IDX_QUICSESSION_STATE_END }; +#undef V #define SESSION_STATS(V) \ V(CREATED_AT, created_at, "Created At") \ @@ -1163,9 +1129,9 @@ class QuicSession : public AsyncWrap, private: static void RandomConnectionIDStrategy( - QuicSession* session, - ngtcp2_cid* cid, - size_t cidlen); + QuicSession* session, + ngtcp2_cid* cid, + size_t cidlen); // Initialize the QuicSession as a server void InitServer( @@ -1219,8 +1185,8 @@ class QuicSession : public AsyncWrap, inline void HandshakeConfirmed(); void PathValidation( - const ngtcp2_path* path, - ngtcp2_path_validation_result res); + const ngtcp2_path* path, + ngtcp2_path_validation_result res); bool ReceivePacket(ngtcp2_path* path, const uint8_t* data, ssize_t nread); @@ -1484,7 +1450,7 @@ class QuicSession : public AsyncWrap, StreamsMap streams_; - AliasedFloat64Array state_; + AliasedStruct state_; struct RemoteTransportParamsDebug { QuicSession* session; From d90c2e7deedfef32fc332871d8637e3a0140d6bb Mon Sep 17 00:00:00 2001 From: James M Snell Date: Thu, 2 Jul 2020 07:07:14 -0700 Subject: [PATCH 06/11] quic: remove onSessionDestroy callback The QuicSession can be destroyed during garbage collection and the onSessionDestroy callback was happening in the destructor. --- lib/internal/quic/core.js | 30 ++++--------------- src/env.h | 1 - src/quic/node_quic.cc | 1 - src/quic/node_quic_session.cc | 17 +---------- src/quic/node_quic_session.h | 2 -- .../test-quic-maxconnectionsperhost.js | 4 +-- 6 files changed, 9 insertions(+), 46 deletions(-) diff --git a/lib/internal/quic/core.js b/lib/internal/quic/core.js index 00907398b09b8d..babbdb39bcf6d9 100644 --- a/lib/internal/quic/core.js +++ b/lib/internal/quic/core.js @@ -288,21 +288,6 @@ function onSessionClose(code, family, silent, statelessReset) { this[owner_symbol][kDestroy](code, family, silent, statelessReset); } -// Called by the C++ internals when a QuicSession has been destroyed. -// When this is called, the QuicSession is no longer usable. Removing -// the handle and emitting close is the only action. -// TODO(@jasnell): In the future, this will need to act differently -// for QuicClientSessions when autoResume is enabled. -function onSessionDestroyed() { - const session = this[owner_symbol]; - this[owner_symbol] = undefined; - - if (session) { - session[kSetHandle](); - process.nextTick(emit.bind(session, 'close')); - } -} - // Used only within the onSessionClientHello function. Invoked // to complete the client hello process. function clientHelloCallback(err, ...args) { @@ -558,7 +543,6 @@ setCallbacks({ onSessionCert, onSessionClientHello, onSessionClose, - onSessionDestroyed, onSessionHandshake, onSessionKeylog, onSessionQlog, @@ -1908,13 +1892,8 @@ class QuicSession extends EventEmitter { this.removeListener('newListener', onNewListener); this.removeListener('removeListener', onRemoveListener); - // If we are destroying with an error, schedule the - // error to be emitted on process.nextTick. - if (error) process.nextTick(emit.bind(this, 'error', error)); - const handle = this[kHandle]; - this[kHandle] = undefined; - + this[kHandle] = undefined if (handle !== undefined) { // Copy the stats for use after destruction state.stats = new BigInt64Array(handle.stats); @@ -1922,14 +1901,17 @@ class QuicSession extends EventEmitter { // Destroy the underlying QuicSession handle handle.destroy(state.closeCode, state.closeFamily); - } else { - process.nextTick(emit.bind(this, 'close')); } // Remove the QuicSession JavaScript object from the // associated QuicSocket. state.socket[kRemoveSession](this); state.socket = undefined; + + // If we are destroying with an error, schedule the + // error to be emitted on process.nextTick. + if (error) process.nextTick(emit.bind(this, 'error', error)); + process.nextTick(emit.bind(this, 'close')); } // For server QuicSession instances, true if earlyData is diff --git a/src/env.h b/src/env.h index f16c79b500171c..95623ab85546f0 100644 --- a/src/env.h +++ b/src/env.h @@ -454,7 +454,6 @@ constexpr size_t kFsStatsBufferLength = V(quic_on_session_cert_function, v8::Function) \ V(quic_on_session_client_hello_function, v8::Function) \ V(quic_on_session_close_function, v8::Function) \ - V(quic_on_session_destroyed_function, v8::Function) \ V(quic_on_session_handshake_function, v8::Function) \ V(quic_on_session_keylog_function, v8::Function) \ V(quic_on_session_path_validation_function, v8::Function) \ diff --git a/src/quic/node_quic.cc b/src/quic/node_quic.cc index 49aba2e5db2870..a651c2f951ffdf 100644 --- a/src/quic/node_quic.cc +++ b/src/quic/node_quic.cc @@ -59,7 +59,6 @@ void QuicSetCallbacks(const FunctionCallbackInfo& args) { SETFUNCTION("onSessionCert", session_cert); SETFUNCTION("onSessionClientHello", session_client_hello); SETFUNCTION("onSessionClose", session_close); - SETFUNCTION("onSessionDestroyed", session_destroyed); SETFUNCTION("onSessionHandshake", session_handshake); SETFUNCTION("onSessionKeylog", session_keylog); SETFUNCTION("onSessionUsePreferredAddress", session_use_preferred_address); diff --git a/src/quic/node_quic_session.cc b/src/quic/node_quic_session.cc index ffc4f25bfcc9d8..0ac7d2ec6fa2a2 100644 --- a/src/quic/node_quic_session.cc +++ b/src/quic/node_quic_session.cc @@ -289,11 +289,6 @@ void QuicSessionListener::OnStreamReset( previous_listener_->OnStreamReset(stream_id, app_error_code); } -void QuicSessionListener::OnSessionDestroyed() { - if (previous_listener_ != nullptr) - previous_listener_->OnSessionDestroyed(); -} - void QuicSessionListener::OnSessionClose(QuicError error, int flags) { if (previous_listener_ != nullptr) previous_listener_->OnSessionClose(error, flags); @@ -509,16 +504,6 @@ void JSQuicSessionListener::OnStreamReset( argv); } -void JSQuicSessionListener::OnSessionDestroyed() { - Environment* env = session()->env(); - HandleScope scope(env->isolate()); - Context::Scope context_scope(env->context()); - // Emit the 'close' event in JS. This needs to happen after destroying the - // connection, because doing so also releases the last qlog data. - session()->MakeCallback( - env->quic_on_session_destroyed_function(), 0, nullptr); -} - void JSQuicSessionListener::OnSessionClose(QuicError error, int flags) { Environment* env = session()->env(); HandleScope scope(env->isolate()); @@ -1412,6 +1397,7 @@ QuicSession::QuicSession( QuicCID(), options, preferred_address_strategy) { + set_wrapped(); InitClient( local_addr, remote_addr, @@ -1472,7 +1458,6 @@ QuicSession::~QuicSession() { CHECK(!Ngtcp2CallbackScope::InNgtcp2CallbackScope(this)); QuicSessionListener* listener_ = listener(); - listener_->OnSessionDestroyed(); if (listener_ == listener()) RemoveListener(listener_); diff --git a/src/quic/node_quic_session.h b/src/quic/node_quic_session.h index acf433622b38d3..7556e2fc5878a3 100644 --- a/src/quic/node_quic_session.h +++ b/src/quic/node_quic_session.h @@ -251,7 +251,6 @@ class QuicSessionListener { virtual void OnStreamReset( int64_t stream_id, uint64_t app_error_code); - virtual void OnSessionDestroyed(); virtual void OnSessionClose( QuicError error, int flags = SESSION_CLOSE_FLAG_NONE); @@ -299,7 +298,6 @@ class JSQuicSessionListener : public QuicSessionListener { void OnStreamReset( int64_t stream_id, uint64_t app_error_code) override; - void OnSessionDestroyed() override; void OnSessionClose( QuicError error, int flags = SESSION_CLOSE_FLAG_NONE) override; diff --git a/test/parallel/test-quic-maxconnectionsperhost.js b/test/parallel/test-quic-maxconnectionsperhost.js index b94849b88272c3..1bfbb7b8e790ea 100644 --- a/test/parallel/test-quic-maxconnectionsperhost.js +++ b/test/parallel/test-quic-maxconnectionsperhost.js @@ -77,8 +77,8 @@ const kALPN = 'zzz'; countdown.dec(); // Shutdown the remaining open sessions. setImmediate(common.mustCall(() => { - for (const req of sessions) - req.close(); + for (const req of sessions) + req.close(); })); })); From 676e89f5131b58e4ec72183b44032ebe10a57c77 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Thu, 2 Jul 2020 12:44:57 -0700 Subject: [PATCH 07/11] quic: refactor qlog handling Because of the timing of qlog events emitted by ngtcp2, it becomes difficult to handle those as events on the QuicSession object because the final qlog entry is not emitted until the ngtcp2_conn is freed, which can occur when the object is being garbage collected (meaning, we a: can't call out to javascript and b: don't have an object we can use to emit the event). This refactors it into a QLogStream object that allows the qlog data to be piped out using a separate Readable stream. --- doc/api/quic.md | 45 +++++--- lib/internal/quic/core.js | 59 +++++----- lib/internal/quic/util.js | 38 +++++++ src/async_wrap.h | 1 + src/env.h | 1 + src/quic/node_quic_session.cc | 107 ++++++++++++++++-- src/quic/node_quic_session.h | 42 ++++++- test/parallel/test-quic-qlog.js | 22 +++- test/sequential/test-async-wrap-getasyncid.js | 1 + 9 files changed, 252 insertions(+), 64 deletions(-) diff --git a/doc/api/quic.md b/doc/api/quic.md index 42a8b756909d1e..3667a05b394956 100644 --- a/doc/api/quic.md +++ b/doc/api/quic.md @@ -274,7 +274,7 @@ added: REPLACEME * `maxStatelessResetsPerHost` {number} The maximum number of stateless resets that the `QuicSocket` is permitted to send per remote host. Default: `10`. - * `qlog` {boolean} Whether to emit ['qlog'][] events for incoming sessions. + * `qlog` {boolean} Whether to enable ['qlog'][] for incoming sessions. (For outgoing client sessions, set `client.qlog`.) Default: `false`. * `retryTokenTimeout` {number} The maximum number of *seconds* for retry token validation. Default: `10` seconds. @@ -633,20 +633,6 @@ The callback will be invoked with three arguments: The `'pathValidation'` event will be emitted multiple times. -#### Event: `'qlog'` - - -* `jsonChunk` {string} A JSON fragment. - -Emitted if the `qlog: true` option was passed to `quicsocket.connect()` or -`net.createQuicSocket()` functions. - -The argument is a JSON fragment according to the [qlog standard][]. - -The `'qlog'` event will be emitted multiple times. - #### Event: `'secure'` + +* Type: {stream.Readable} + +If `qlog` support is enabled for `QuicSession`, the `quicsession.qlog` property +provides a [`stream.Readable`][] that may be used to access the `qlog` event +data according to the [qlog standard][]. For client `QuicSessions`, the +`quicsession.qlog` property will be `undefined` untilt the `'qlog'` event +is emitted. + #### quicsession.remoteAddress + +The `'qlog'` event is emitted when the `QuicClientSession` is ready to begin +providing `qlog` event data. The callback is invoked with a single argument: + +* `qlog` {stream.Readable} A [`stream.Readable`][] that is also available using + the `quicsession.qlog` property. + #### Event: `'usePreferredAddress'`