quic: apply multiple additional minor improvements

Signed-off-by: James M Snell <jasnell@gmail.com>
PR-URL: #63267
Backport-PR-URL: #64675
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() {
 }
 
 int Endpoint::UDP::Bind(const Options& options) {
-  if (is_bound_) return UV_EALREADY;
+  if (flags_.is_bound) return UV_EALREADY;
   if (is_closed_or_closing()) return UV_EBADF;
 
   int flags = 0;
@@ -389,7 +389,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_),
@@ -428,34 +428,34 @@ void Endpoint::UDP::Unref() {
 
 int Endpoint::UDP::Start() {
   if (is_closed_or_closing()) return UV_EBADF;
-  if (is_started_) return 0;
+  if (flags_.is_started) return 0;
   int err = uv_udp_recv_start(&impl_->handle_, Impl::OnAlloc, Impl::OnReceive);
-  is_started_ = (err == 0);
+  flags_.is_started = (err == 0);
   return err;
 }
 
 void Endpoint::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;
 }
 
 void Endpoint::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();
 }
 
 bool Endpoint::UDP::is_bound() const {
-  return is_bound_;
+  return flags_.is_bound;
 }
 
 bool Endpoint::UDP::is_closed() const {
-  return is_closed_;
+  return flags_.is_closed;
 }
 
 bool Endpoint::UDP::is_closed_or_closing() const {
@@ -1295,8 +1295,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

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

src/quic/quic.cc

@@ -13,7 +13,7 @@
 #include "node_external_reference.h"
 
 #include <ngtcp2/ngtcp2_crypto_ossl.h>
-#include <mutex>
+
 namespace node {
 
 using v8::Context;
@@ -25,7 +25,11 @@ using v8::Value;
 namespace quic {
 
 namespace {
-std::once_flag crypto_init_flag;
+uv_once_t crypto_init_flag = UV_ONCE_INIT;
+
+void InitNgtcp2CryptoOnce() {
+  ngtcp2_crypto_ossl_init();
+}
 }  // namespace
 
 void CreatePerIsolateProperties(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

@@ -1818,7 +1818,7 @@ bool Session::is_server() const {
 }
 
 bool Session::is_destroyed() const {
-  return !impl_ || destroy_deferred_;
+  return !impl_ || flags_.destroy_deferred;
 }
 
 bool Session::is_destroyed_or_closing() const {
@@ -1996,9 +1996,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;
   }
 
@@ -2139,8 +2139,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),
@@ -2389,7 +2389,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.
@@ -2404,9 +2404,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;
   }
 }
 
@@ -2430,7 +2430,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;
@@ -2865,7 +2865,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;
+
+  struct Flags {
+    // 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_;
   friend struct NgTcp2CallbackScope;