quic: apply multiple additional minor improvements

Signed-off-by: James M Snell <jasnell@gmail.com>
PR-URL: #63267
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Author: jasnell

src/quic/bindingdata.cc

@@ -335,7 +335,7 @@ void BindingData::OnFlushCheck() {
 // pending_flush_sessions_ and are picked up on the next check tick.
 auto sessions = std::move(pending_flush_sessions_);
 for (auto& session : sessions) {
- session->pending_flush_ = false;
+ session->flags_.pending_flush = false;
 if (!session->is_destroyed()) {
  session->FlushPendingData();
  }
@@ -437,28 +437,28 @@ JS_METHOD_IMPL(BindingData::SetCallbacks) {
 }
 
 NgTcp2CallbackScope::NgTcp2CallbackScope(Session* session) : session(session) {
-CHECK(!session->in_ngtcp2_callback_scope_);
- session->in_ngtcp2_callback_scope_ = true;
+CHECK(!session->flags_.in_ngtcp2_callback_scope);
+ session->flags_.in_ngtcp2_callback_scope = true;
 }
 
 NgTcp2CallbackScope::~NgTcp2CallbackScope() {
- session->in_ngtcp2_callback_scope_ = false;
-if (session->destroy_deferred_) {
- session->destroy_deferred_ = false;
+ session->flags_.in_ngtcp2_callback_scope = false;
+if (session->flags_.destroy_deferred) {
+ session->flags_.destroy_deferred = false;
  session->Destroy();
  }
 }
 
 NgHttp3CallbackScope::NgHttp3CallbackScope(Session* session)
  : session(session) {
-CHECK(!session->in_nghttp3_callback_scope_);
- session->in_nghttp3_callback_scope_ = true;
+CHECK(!session->flags_.in_nghttp3_callback_scope);
+ session->flags_.in_nghttp3_callback_scope = true;
 }
 
 NgHttp3CallbackScope::~NgHttp3CallbackScope() {
- session->in_nghttp3_callback_scope_ = false;
-if (session->destroy_deferred_) {
- session->destroy_deferred_ = false;
+ session->flags_.in_nghttp3_callback_scope = false;
+if (session->flags_.destroy_deferred) {
+ session->flags_.destroy_deferred = false;
  session->Destroy();
  }
 }

src/quic/data.cc

@@ -137,7 +137,9 @@ Store Store::CopyFrom(Local<ArrayBuffer> buffer) {
 auto backing = buffer->GetBackingStore();
 auto length = buffer->ByteLength();
 auto dest = ArrayBuffer::NewBackingStore(
- isolate, length, BackingStoreInitializationMode::kUninitialized,
+ isolate,
+ length,
+ BackingStoreInitializationMode::kUninitialized,
  BackingStoreOnFailureMode::kReturnNull);
 if (!dest) {
 THROW_ERR_MEMORY_ALLOCATION_FAILED(Environment::GetCurrent(isolate));
@@ -154,7 +156,9 @@ Store Store::CopyFrom(Local<ArrayBufferView> view) {
 auto length = view->ByteLength();
 auto offset = view->ByteOffset();
 auto dest = ArrayBuffer::NewBackingStore(
- isolate, length, BackingStoreInitializationMode::kUninitialized,
+ isolate,
+ length,
+ BackingStoreInitializationMode::kUninitialized,
  BackingStoreOnFailureMode::kReturnNull);
 // copy content
 if (!dest) {

src/quic/endpoint.cc

@@ -378,7 +378,7 @@ Endpoint::UDP::~UDP() {
 }
 
 intEndpoint::UDP::Bind(const Options& options) {
-if (is_bound_) returnUV_EALREADY;
+if (flags_.is_bound) returnUV_EALREADY;
 if (is_closed_or_closing()) returnUV_EBADF;
 
 int flags = 0;
@@ -388,7 +388,7 @@ int Endpoint::UDP::Bind(const Options& options) {
 int size;
 
 if (!err) {
-is_bound_ = true;
+flags_.is_bound = true;
  size = static_cast<int>(options.udp_receive_buffer_size);
 if (size > 0) {
  err = uv_recv_buffer_size(reinterpret_cast<uv_handle_t*>(&impl_->handle_),
@@ -427,34 +427,34 @@ void Endpoint::UDP::Unref() {
 
 intEndpoint::UDP::Start() {
 if (is_closed_or_closing()) returnUV_EBADF;
-if (is_started_) return0;
+if (flags_.is_started) return0;
 int err = uv_udp_recv_start(&impl_->handle_, Impl::OnAlloc, Impl::OnReceive);
-is_started_ = (err == 0);
+flags_.is_started = (err == 0);
 return err;
 }
 
 voidEndpoint::UDP::Stop() {
-if (is_closed_or_closing() || !is_started_) return;
+if (is_closed_or_closing() || !flags_.is_started) return;
 USE(uv_udp_recv_stop(&impl_->handle_));
-is_started_ = false;
+flags_.is_started = false;
 }
 
 voidEndpoint::UDP::Close() {
 if (is_closed_or_closing()) return;
 DCHECK(impl_);
 Stop();
-is_bound_ = false;
-is_closed_ = true;
+flags_.is_bound = false;
+flags_.is_closed = true;
  impl_->Close();
  impl_.reset();
 }
 
 boolEndpoint::UDP::is_bound() const {
-returnis_bound_;
+returnflags_.is_bound;
 }
 
 boolEndpoint::UDP::is_closed() const {
-returnis_closed_;
+returnflags_.is_closed;
 }
 
 boolEndpoint::UDP::is_closed_or_closing() const {
@@ -1294,8 +1294,8 @@ void Endpoint::Receive(const uint8_t* data,
  }
 // Schedule the session for deferred SendPendingData if it hasn't
 // been scheduled already in this burst.
-if (!session->is_destroyed() && !session->pending_flush_) {
- session->pending_flush_ = true;
+if (!session->is_destroyed() && !session->flags_.pending_flush) {
+ session->flags_.pending_flush = true;
 BindingData::Get(env()).ScheduleSessionFlush(
  BaseObjectPtr<Session>(session));
  }

src/quic/endpoint.h

@@ -343,9 +343,12 @@ class Endpoint final : public AsyncWrap, public Packet::Listener {
 classImpl;
 
  BaseObjectWeakPtr<Impl> impl_;
-bool is_bound_ = false;
-bool is_started_ = false;
-bool is_closed_ = false;
+structFlags {
+uint8_t is_bound : 1 = 0;
+uint8_t is_started : 1 = 0;
+uint8_t is_closed : 1 = 0;
+ };
+ Flags flags_;
  };
 
 boolis_closed() const;

src/quic/quic.cc

@@ -13,7 +13,7 @@
 #include"node_external_reference.h"
 
 #include<ngtcp2/ngtcp2_crypto_ossl.h>
-#include<mutex>
+
 namespacenode {
 
 using v8::Context;
@@ -25,7 +25,11 @@ using v8::Value;
 namespacequic {
 
 namespace {
-std::once_flag crypto_init_flag;
+uv_once_t crypto_init_flag = UV_ONCE_INIT;
+
+voidInitNgtcp2CryptoOnce() {
+ngtcp2_crypto_ossl_init();
+}
 } // namespace
 
 voidCreatePerIsolateProperties(IsolateData* isolate_data,
@@ -39,8 +43,8 @@ void CreatePerContextProperties(Local<Object> target,
  Local<Value> unused,
  Local<Context> context,
 void* priv) {
+uv_once(&crypto_init_flag, InitNgtcp2CryptoOnce);
  Realm* realm = Realm::GetCurrent(context);
-std::call_once(crypto_init_flag, ngtcp2_crypto_ossl_init);
 BindingData::InitPerContext(realm, target);
 Endpoint::InitPerContext(realm, target);
 Session::InitPerContext(realm, target);

src/quic/session.cc

@@ -1810,7 +1810,7 @@ bool Session::is_server() const {
 }
 
 boolSession::is_destroyed() const {
-return !impl_ || destroy_deferred_;
+return !impl_ || flags_.destroy_deferred;
 }
 
 boolSession::is_destroyed_or_closing() const {
@@ -1988,9 +1988,9 @@ void Session::Destroy() {
 // destroy impl_ now because the callback is executing methods on
 // objects owned by impl_ (e.g., the Application). Defer the
 // destruction until the scope exits.
-if (in_ngtcp2_callback_scope_ || in_nghttp3_callback_scope_) {
+if (flags_.in_ngtcp2_callback_scope || flags_.in_nghttp3_callback_scope) {
 Debug(this, "Session destroy deferred (in callback scope)");
-destroy_deferred_ = true;
+flags_.destroy_deferred = true;
 return;
  }
 
@@ -2131,8 +2131,8 @@ void Session::EmitQlog(uint32_t flags, std::string_view data) {
 if (is_destroyed()) {
 auto isolate = env()->isolate();
  Global<Object> recv(isolate, object());
- Global<Function> cb(
-isolate,BindingData::Get(env()).session_qlog_callback());
+ Global<Function> cb(isolate,
+BindingData::Get(env()).session_qlog_callback());
  std::string buf(data);
 env()->SetImmediate([recv = std::move(recv),
  cb = std::move(cb),
@@ -2381,7 +2381,7 @@ void Session::SendBatch(Packet::Ptr* packets,
 if (primary_count == 0) return;
 
 // Use batched send for the primary endpoint.
-if (prefer_try_send_) {
+if (flags_.prefer_try_send) {
 endpoint().SendBatch(primary_packets, primary_count);
  } else {
 // Non-flush path: send individually via async uv_udp_send.
@@ -2396,9 +2396,9 @@ void Session::FlushPendingData() {
 if (impl_->application_) {
 // Prefer synchronous sends during the deferred flush to avoid the
 // one-tick latency of async uv_udp_send from the uv_check callback.
-prefer_try_send_ = true;
+flags_.prefer_try_send = true;
 application().SendPendingData();
-prefer_try_send_ = false;
+flags_.prefer_try_send = false;
  }
 }
 
@@ -2422,7 +2422,7 @@ void Session::Send(Packet::Ptr packet) {
 // prefer synchronous send to avoid the one-tick latency of async
 // uv_udp_send. SendOrTrySend uses uv_udp_try_send first, falling
 // back to uv_udp_send on EAGAIN.
-if (prefer_try_send_) {
+if (flags_.prefer_try_send) {
 Debug(this, "Session is sending (try_send) %s", packet->ToString());
 endpoint().SendOrTrySend(std::move(packet));
 return;
@@ -2857,7 +2857,7 @@ bool Session::can_send_packets() const {
 // or closing period. The callback scope check is per-session so that
 // one session's ngtcp2 callback does not block unrelated sessions
 // from sending.
-return !is_destroyed() && !in_ngtcp2_callback_scope_ &&
+return !is_destroyed() && !flags_.in_ngtcp2_callback_scope &&
  !is_in_draining_period() && !is_in_closing_period();
 }
 

src/quic/session.h

@@ -606,25 +606,30 @@ class Session final : public AsyncWrap, private SessionTicket::AppData::Source {
  Side side_;
 const ngtcp2_mem* allocator_;
  std::unique_ptr<Impl> impl_;
-// These flags live on Session (not Impl) so that the NgTcp2CallbackScope
-// and NgHttp3CallbackScope destructors can safely clear them even after
-// Impl has been destroyed via MakeCallback re-entrancy during a callback.
-// The scope is placed at the ngtcp2/nghttp3 entry point (e.g. Receive,
-// OnTimeout) rather than on individual callbacks, so the deferred destroy
-// only fires after all callbacks for that entry point have completed.
-bool in_ngtcp2_callback_scope_ = false;
-bool in_nghttp3_callback_scope_ = false;
-bool destroy_deferred_ = false;
-// Set when this session is in BindingData's pending_flush_sessions_ vector.
-// Cleared by the flush callback before calling SendPendingData.
-// Provides O(1) dedup so a session receiving multiple packets in one I/O
-// burst is only scheduled for flush once.
-bool pending_flush_ = false;
-// When true, Session::Send prefers synchronous delivery via
-// Endpoint::SendOrTrySend (uv_udp_try_send with async fallback).
-// Set during FlushPendingData to avoid the one-tick latency of
-// async-only sends from the uv_check callback.
-bool prefer_try_send_ = false;
+
+structFlags {
+// These flags live on Session (not Impl) so that the NgTcp2CallbackScope
+// and NgHttp3CallbackScope destructors can safely clear them even after
+// Impl has been destroyed via MakeCallback re-entrancy during a callback.
+// The scope is placed at the ngtcp2/nghttp3 entry point (e.g. Receive,
+// OnTimeout) rather than on individual callbacks, so the deferred destroy
+// only fires after all callbacks for that entry point have completed.
+uint8_t in_ngtcp2_callback_scope : 1 = 0;
+uint8_t in_nghttp3_callback_scope : 1 = 0;
+uint8_t destroy_deferred : 1 = 0;
+// Set when this session is in BindingData's pending_flush_sessions_ vector.
+// Cleared by the flush callback before calling SendPendingData.
+// Provides O(1) dedup so a session receiving multiple packets in one I/O
+// burst is only scheduled for flush once.
+uint8_t pending_flush : 1 = 0;
+// When true, Session::Send prefers synchronous delivery via
+// Endpoint::SendOrTrySend (uv_udp_try_send with async fallback).
+// Set during FlushPendingData to avoid the one-tick latency of
+// async-only sends from the uv_check callback.
+uint8_t prefer_try_send : 1 = 0;
+ };
+ Flags flags_;
+
  QuicConnectionPointer connection_;
  std::unique_ptr<TLSSession> tls_session_;
 friendstructNgTcp2CallbackScope;