FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

quic: apply multiple additional minor improvements · nodejs/node@f727efb · GitHub

/ node Public

Commit f727efb

Browse files
jasnell
authored and
aduh95
committed
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>
1 parent ee0a0f1 commit f727efb

10 files changed

Lines changed: 109 additions & 92 deletions

File tree

‎src/quic/bindingdata.cc‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,7 @@ void BindingData::OnFlushCheck() {
335335
// pending_flush_sessions_ and are picked up on the next check tick.
336336
auto sessions = std::move(pending_flush_sessions_);
337337
for (auto& session : sessions) {
338-
session->pending_flush_ = false;
338+
session->flags_.pending_flush = false;
339339
if (!session->is_destroyed()) {
340340
session->FlushPendingData();
341341
}
@@ -437,28 +437,28 @@ JS_METHOD_IMPL(BindingData::SetCallbacks) {
437437
}
438438

439439
NgTcp2CallbackScope::NgTcp2CallbackScope(Session* session) : session(session) {
440-
CHECK(!session->in_ngtcp2_callback_scope_);
441-
session->in_ngtcp2_callback_scope_ = true;
440+
CHECK(!session->flags_.in_ngtcp2_callback_scope);
441+
session->flags_.in_ngtcp2_callback_scope = true;
442442
}
443443

444444
NgTcp2CallbackScope::~NgTcp2CallbackScope() {
445-
session->in_ngtcp2_callback_scope_ = false;
446-
if (session->destroy_deferred_) {
447-
session->destroy_deferred_ = false;
445+
session->flags_.in_ngtcp2_callback_scope = false;
446+
if (session->flags_.destroy_deferred) {
447+
session->flags_.destroy_deferred = false;
448448
session->Destroy();
449449
}
450450
}
451451

452452
NgHttp3CallbackScope::NgHttp3CallbackScope(Session* session)
453453
: session(session) {
454-
CHECK(!session->in_nghttp3_callback_scope_);
455-
session->in_nghttp3_callback_scope_ = true;
454+
CHECK(!session->flags_.in_nghttp3_callback_scope);
455+
session->flags_.in_nghttp3_callback_scope = true;
456456
}
457457

458458
NgHttp3CallbackScope::~NgHttp3CallbackScope() {
459-
session->in_nghttp3_callback_scope_ = false;
460-
if (session->destroy_deferred_) {
461-
session->destroy_deferred_ = false;
459+
session->flags_.in_nghttp3_callback_scope = false;
460+
if (session->flags_.destroy_deferred) {
461+
session->flags_.destroy_deferred = false;
462462
session->Destroy();
463463
}
464464
}

‎src/quic/data.cc‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,9 @@ Store Store::CopyFrom(Local<ArrayBuffer> buffer) {
137137
auto backing = buffer->GetBackingStore();
138138
auto length = buffer->ByteLength();
139139
auto dest = ArrayBuffer::NewBackingStore(
140-
isolate, length, BackingStoreInitializationMode::kUninitialized,
140+
isolate,
141+
length,
142+
BackingStoreInitializationMode::kUninitialized,
141143
BackingStoreOnFailureMode::kReturnNull);
142144
if (!dest) {
143145
THROW_ERR_MEMORY_ALLOCATION_FAILED(Environment::GetCurrent(isolate));
@@ -154,7 +156,9 @@ Store Store::CopyFrom(Local<ArrayBufferView> view) {
154156
auto length = view->ByteLength();
155157
auto offset = view->ByteOffset();
156158
auto dest = ArrayBuffer::NewBackingStore(
157-
isolate, length, BackingStoreInitializationMode::kUninitialized,
159+
isolate,
160+
length,
161+
BackingStoreInitializationMode::kUninitialized,
158162
BackingStoreOnFailureMode::kReturnNull);
159163
// copy content
160164
if (!dest) {

‎src/quic/endpoint.cc‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -378,7 +378,7 @@ Endpoint::UDP::~UDP() {
378378
}
379379

380380
int Endpoint::UDP::Bind(const Options& options) {
381-
if (is_bound_) return UV_EALREADY;
381+
if (flags_.is_bound) return UV_EALREADY;
382382
if (is_closed_or_closing()) return UV_EBADF;
383383

384384
int flags = 0;
@@ -389,7 +389,7 @@ int Endpoint::UDP::Bind(const Options& options) {
389389
int size;
390390

391391
if (!err) {
392-
is_bound_ = true;
392+
flags_.is_bound = true;
393393
size = static_cast<int>(options.udp_receive_buffer_size);
394394
if (size > 0) {
395395
err = uv_recv_buffer_size(reinterpret_cast<uv_handle_t*>(&impl_->handle_),
@@ -428,34 +428,34 @@ void Endpoint::UDP::Unref() {
428428

429429
int Endpoint::UDP::Start() {
430430
if (is_closed_or_closing()) return UV_EBADF;
431-
if (is_started_) return 0;
431+
if (flags_.is_started) return 0;
432432
int err = uv_udp_recv_start(&impl_->handle_, Impl::OnAlloc, Impl::OnReceive);
433-
is_started_ = (err == 0);
433+
flags_.is_started = (err == 0);
434434
return err;
435435
}
436436

437437
void Endpoint::UDP::Stop() {
438-
if (is_closed_or_closing() || !is_started_) return;
438+
if (is_closed_or_closing() || !flags_.is_started) return;
439439
USE(uv_udp_recv_stop(&impl_->handle_));
440-
is_started_ = false;
440+
flags_.is_started = false;
441441
}
442442

443443
void Endpoint::UDP::Close() {
444444
if (is_closed_or_closing()) return;
445445
DCHECK(impl_);
446446
Stop();
447-
is_bound_ = false;
448-
is_closed_ = true;
447+
flags_.is_bound = false;
448+
flags_.is_closed = true;
449449
impl_->Close();
450450
impl_.reset();
451451
}
452452

453453
bool Endpoint::UDP::is_bound() const {
454-
return is_bound_;
454+
return flags_.is_bound;
455455
}
456456

457457
bool Endpoint::UDP::is_closed() const {
458-
return is_closed_;
458+
return flags_.is_closed;
459459
}
460460

461461
bool Endpoint::UDP::is_closed_or_closing() const {
@@ -1295,8 +1295,8 @@ void Endpoint::Receive(const uint8_t* data,
12951295
}
12961296
// Schedule the session for deferred SendPendingData if it hasn't
12971297
// been scheduled already in this burst.
1298-
if (!session->is_destroyed() && !session->pending_flush_) {
1299-
session->pending_flush_ = true;
1298+
if (!session->is_destroyed() && !session->flags_.pending_flush) {
1299+
session->flags_.pending_flush = true;
13001300
BindingData::Get(env()).ScheduleSessionFlush(
13011301
BaseObjectPtr<Session>(session));
13021302
}

‎src/quic/endpoint.h‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -349,9 +349,12 @@ class Endpoint final : public AsyncWrap, public Packet::Listener {
349349
class Impl;
350350

351351
BaseObjectWeakPtr<Impl> impl_;
352-
bool is_bound_ = false;
353-
bool is_started_ = false;
354-
bool is_closed_ = false;
352+
struct Flags {
353+
uint8_t is_bound : 1 = 0;
354+
uint8_t is_started : 1 = 0;
355+
uint8_t is_closed : 1 = 0;
356+
};
357+
Flags flags_;
355358
};
356359

357360
bool is_closed() const;

‎src/quic/quic.cc‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
#include "node_external_reference.h"
1414

1515
#include <ngtcp2/ngtcp2_crypto_ossl.h>
16-
#include <mutex>
16+
1717
namespace node {
1818

1919
using v8::Context;
@@ -25,7 +25,11 @@ using v8::Value;
2525
namespace quic {
2626

2727
namespace {
28-
std::once_flag crypto_init_flag;
28+
uv_once_t crypto_init_flag = UV_ONCE_INIT;
29+
30+
void InitNgtcp2CryptoOnce() {
31+
ngtcp2_crypto_ossl_init();
32+
}
2933
} // namespace
3034

3135
void CreatePerIsolateProperties(IsolateData* isolate_data,
@@ -39,8 +43,8 @@ void CreatePerContextProperties(Local<Object> target,
3943
Local<Value> unused,
4044
Local<Context> context,
4145
void* priv) {
46+
uv_once(&crypto_init_flag, InitNgtcp2CryptoOnce);
4247
Realm* realm = Realm::GetCurrent(context);
43-
std::call_once(crypto_init_flag, ngtcp2_crypto_ossl_init);
4448
BindingData::InitPerContext(realm, target);
4549
Endpoint::InitPerContext(realm, target);
4650
Session::InitPerContext(realm, target);

‎src/quic/session.cc‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1818,7 +1818,7 @@ bool Session::is_server() const {
18181818
}
18191819

18201820
bool Session::is_destroyed() const {
1821-
return !impl_ || destroy_deferred_;
1821+
return !impl_ || flags_.destroy_deferred;
18221822
}
18231823

18241824
bool Session::is_destroyed_or_closing() const {
@@ -1996,9 +1996,9 @@ void Session::Destroy() {
19961996
// destroy impl_ now because the callback is executing methods on
19971997
// objects owned by impl_ (e.g., the Application). Defer the
19981998
// destruction until the scope exits.
1999-
if (in_ngtcp2_callback_scope_ || in_nghttp3_callback_scope_) {
1999+
if (flags_.in_ngtcp2_callback_scope || flags_.in_nghttp3_callback_scope) {
20002000
Debug(this, "Session destroy deferred (in callback scope)");
2001-
destroy_deferred_ = true;
2001+
flags_.destroy_deferred = true;
20022002
return;
20032003
}
20042004

@@ -2139,8 +2139,8 @@ void Session::EmitQlog(uint32_t flags, std::string_view data) {
21392139
if (is_destroyed()) {
21402140
auto isolate = env()->isolate();
21412141
Global<Object> recv(isolate, object());
2142-
Global<Function> cb(
2143-
isolate, BindingData::Get(env()).session_qlog_callback());
2142+
Global<Function> cb(isolate,
2143+
BindingData::Get(env()).session_qlog_callback());
21442144
std::string buf(data);
21452145
env()->SetImmediate([recv = std::move(recv),
21462146
cb = std::move(cb),
@@ -2389,7 +2389,7 @@ void Session::SendBatch(Packet::Ptr* packets,
23892389
if (primary_count == 0) return;
23902390

23912391
// Use batched send for the primary endpoint.
2392-
if (prefer_try_send_) {
2392+
if (flags_.prefer_try_send) {
23932393
endpoint().SendBatch(primary_packets, primary_count);
23942394
} else {
23952395
// Non-flush path: send individually via async uv_udp_send.
@@ -2404,9 +2404,9 @@ void Session::FlushPendingData() {
24042404
if (impl_->application_) {
24052405
// Prefer synchronous sends during the deferred flush to avoid the
24062406
// one-tick latency of async uv_udp_send from the uv_check callback.
2407-
prefer_try_send_ = true;
2407+
flags_.prefer_try_send = true;
24082408
application().SendPendingData();
2409-
prefer_try_send_ = false;
2409+
flags_.prefer_try_send = false;
24102410
}
24112411
}
24122412

@@ -2430,7 +2430,7 @@ void Session::Send(Packet::Ptr packet) {
24302430
// prefer synchronous send to avoid the one-tick latency of async
24312431
// uv_udp_send. SendOrTrySend uses uv_udp_try_send first, falling
24322432
// back to uv_udp_send on EAGAIN.
2433-
if (prefer_try_send_) {
2433+
if (flags_.prefer_try_send) {
24342434
Debug(this, "Session is sending (try_send) %s", packet->ToString());
24352435
endpoint().SendOrTrySend(std::move(packet));
24362436
return;
@@ -2865,7 +2865,7 @@ bool Session::can_send_packets() const {
28652865
// or closing period. The callback scope check is per-session so that
28662866
// one session's ngtcp2 callback does not block unrelated sessions
28672867
// from sending.
2868-
return !is_destroyed() && !in_ngtcp2_callback_scope_ &&
2868+
return !is_destroyed() && !flags_.in_ngtcp2_callback_scope &&
28692869
!is_in_draining_period() && !is_in_closing_period();
28702870
}
28712871

‎src/quic/session.h‎

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -606,25 +606,30 @@ class Session final : public AsyncWrap, private SessionTicket::AppData::Source {
606606
Side side_;
607607
const ngtcp2_mem* allocator_;
608608
std::unique_ptr<Impl> impl_;
609-
// These flags live on Session (not Impl) so that the NgTcp2CallbackScope
610-
// and NgHttp3CallbackScope destructors can safely clear them even after
611-
// Impl has been destroyed via MakeCallback re-entrancy during a callback.
612-
// The scope is placed at the ngtcp2/nghttp3 entry point (e.g. Receive,
613-
// OnTimeout) rather than on individual callbacks, so the deferred destroy
614-
// only fires after all callbacks for that entry point have completed.
615-
bool in_ngtcp2_callback_scope_ = false;
616-
bool in_nghttp3_callback_scope_ = false;
617-
bool destroy_deferred_ = false;
618-
// Set when this session is in BindingData's pending_flush_sessions_ vector.
619-
// Cleared by the flush callback before calling SendPendingData.
620-
// Provides O(1) dedup so a session receiving multiple packets in one I/O
621-
// burst is only scheduled for flush once.
622-
bool pending_flush_ = false;
623-
// When true, Session::Send prefers synchronous delivery via
624-
// Endpoint::SendOrTrySend (uv_udp_try_send with async fallback).
625-
// Set during FlushPendingData to avoid the one-tick latency of
626-
// async-only sends from the uv_check callback.
627-
bool prefer_try_send_ = false;
609+
610+
struct Flags {
611+
// These flags live on Session (not Impl) so that the NgTcp2CallbackScope
612+
// and NgHttp3CallbackScope destructors can safely clear them even after
613+
// Impl has been destroyed via MakeCallback re-entrancy during a callback.
614+
// The scope is placed at the ngtcp2/nghttp3 entry point (e.g. Receive,
615+
// OnTimeout) rather than on individual callbacks, so the deferred destroy
616+
// only fires after all callbacks for that entry point have completed.
617+
uint8_t in_ngtcp2_callback_scope : 1 = 0;
618+
uint8_t in_nghttp3_callback_scope : 1 = 0;
619+
uint8_t destroy_deferred : 1 = 0;
620+
// Set when this session is in BindingData's pending_flush_sessions_ vector.
621+
// Cleared by the flush callback before calling SendPendingData.
622+
// Provides O(1) dedup so a session receiving multiple packets in one I/O
623+
// burst is only scheduled for flush once.
624+
uint8_t pending_flush : 1 = 0;
625+
// When true, Session::Send prefers synchronous delivery via
626+
// Endpoint::SendOrTrySend (uv_udp_try_send with async fallback).
627+
// Set during FlushPendingData to avoid the one-tick latency of
628+
// async-only sends from the uv_check callback.
629+
uint8_t prefer_try_send : 1 = 0;
630+
};
631+
Flags flags_;
632+
628633
QuicConnectionPointer connection_;
629634
std::unique_ptr<TLSSession> tls_session_;
630635
friend struct NgTcp2CallbackScope;

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL