Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

quic: multiple cleanups #34137

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
quic: consolidate onSessionClose and onSessionSilentClose
Use a single callback function for both
  • Loading branch information
jasnell committed Jun 30, 2020
commit 0aa82a454c72b7ad4add8b4e01e0f1ecc4d93e64
40 changes: 16 additions & 24 deletions lib/internal/quic/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -271,23 +271,24 @@ function onSessionReady(handle) {
process.nextTick(emit.bind(socket, 'session', session));
}

// During an immediate close, all currently open QuicStreams are
// abruptly closed. If they are still writable or readable, an abort
// event will be emitted, and RESET_STREAM and STOP_SENDING frames
// will be transmitted as necessary. Once streams have been
// shutdown, a CONNECTION_CLOSE frame will be sent and the
// session will enter the closing period, after which it will
// be destroyed either when the idle timeout expires, the
// QuicSession is silently closed, or destroy is called.
function onSessionClose(code, family) {
// 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.
function onSessionClose(code, family, silent, statelessReset) {
if (this[owner_symbol]) {
this[owner_symbol][kClose](family, code);
} else {
// 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);
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);
}

// Called by the C++ internals when a QuicSession has been destroyed.
Expand Down Expand Up @@ -554,14 +555,6 @@ function onStreamHeaders(id, headers, kind, push_id) {
this[owner_symbol][kHeaders](id, headers, kind, push_id);
}

// During a silent close, all currently open QuicStreams are abruptly
// closed. If they are still writable or readable, an abort event will be
// emitted, otherwise the stream is just destroyed. No RESET_STREAM or
// STOP_SENDING is transmitted to the peer.
function onSessionSilentClose(statelessReset, code, family) {
this[owner_symbol][kDestroy](statelessReset, family, code);
}

// When a stream is flow control blocked, causes a blocked event
// to be emitted. This is a purely informational event.
function onStreamBlocked() {
Expand All @@ -581,7 +574,6 @@ setCallbacks({
onSessionHandshake,
onSessionKeylog,
onSessionQlog,
onSessionSilentClose,
onSessionStatus,
onSessionTicket,
onSessionVersionNegotiation,
Expand Down
1 change: 0 additions & 1 deletion src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,6 @@ constexpr size_t kFsStatsBufferLength =
V(quic_on_session_use_preferred_address_function, v8::Function) \
V(quic_on_session_qlog_function, v8::Function) \
V(quic_on_session_ready_function, v8::Function) \
V(quic_on_session_silent_close_function, v8::Function) \
V(quic_on_session_status_function, v8::Function) \
V(quic_on_session_ticket_function, v8::Function) \
V(quic_on_session_version_negotiation_function, v8::Function) \
Expand Down
1 change: 0 additions & 1 deletion src/quic/node_quic.cc
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ void QuicSetCallbacks(const FunctionCallbackInfo<Value>& args) {
SETFUNCTION("onSessionUsePreferredAddress", session_use_preferred_address);
SETFUNCTION("onSessionPathValidation", session_path_validation);
SETFUNCTION("onSessionQlog", session_qlog);
SETFUNCTION("onSessionSilentClose", session_silent_close);
SETFUNCTION("onSessionStatus", session_status);
SETFUNCTION("onSessionTicket", session_ticket);
SETFUNCTION("onSessionVersionNegotiation", session_version_negotiation);
Expand Down
46 changes: 14 additions & 32 deletions src/quic/node_quic_session.cc
Original file line number Diff line number Diff line change
Expand Up @@ -293,9 +293,9 @@ void QuicSessionListener::OnSessionDestroyed() {
previous_listener_->OnSessionDestroyed();
}

void QuicSessionListener::OnSessionClose(QuicError error) {
void QuicSessionListener::OnSessionClose(QuicError error, int flags) {
if (previous_listener_ != nullptr)
previous_listener_->OnSessionClose(error);
previous_listener_->OnSessionClose(error, flags);
}

void QuicSessionListener::OnStreamReady(BaseObjectPtr<QuicStream> stream) {
Expand Down Expand Up @@ -328,13 +328,6 @@ void QuicSessionListener::OnStreamBlocked(int64_t stream_id) {
}
}

void QuicSessionListener::OnSessionSilentClose(
bool stateless_reset,
QuicError error) {
if (previous_listener_ != nullptr)
previous_listener_->OnSessionSilentClose(stateless_reset, error);
}

void QuicSessionListener::OnUsePreferredAddress(
int family,
const PreferredAddress& preferred_address) {
Expand Down Expand Up @@ -525,14 +518,20 @@ void JSQuicSessionListener::OnSessionDestroyed() {
env->quic_on_session_destroyed_function(), 0, nullptr);
}

void JSQuicSessionListener::OnSessionClose(QuicError error) {
void JSQuicSessionListener::OnSessionClose(QuicError error, int flags) {
Environment* env = session()->env();
HandleScope scope(env->isolate());
Context::Scope context_scope(env->context());

Local<Value> argv[] = {
Number::New(env->isolate(), static_cast<double>(error.code)),
Integer::New(env->isolate(), error.family)
Integer::New(env->isolate(), error.family),
flags & SESSION_CLOSE_FLAG_SILENT
? v8::True(env->isolate())
: v8::False(env->isolate()),
flags & SESSION_CLOSE_FLAG_STATELESS_RESET
? v8::True(env->isolate())
: v8::False(env->isolate())
};

// Grab a shared pointer to this to prevent the QuicSession
Expand Down Expand Up @@ -664,26 +663,6 @@ void JSQuicSessionListener::OnSessionTicket(int size, SSL_SESSION* sess) {
arraysize(argv), argv);
}

void JSQuicSessionListener::OnSessionSilentClose(
bool stateless_reset,
QuicError error) {
Environment* env = session()->env();
HandleScope scope(env->isolate());
Context::Scope context_scope(env->context());

Local<Value> argv[] = {
stateless_reset ? v8::True(env->isolate()) : v8::False(env->isolate()),
Number::New(env->isolate(), static_cast<double>(error.code)),
Integer::New(env->isolate(), error.family)
};

// Grab a shared pointer to this to prevent the QuicSession
// from being freed while the MakeCallback is running.
BaseObjectPtr<QuicSession> ptr(session());
session()->MakeCallback(
env->quic_on_session_silent_close_function(), arraysize(argv), argv);
}

void JSQuicSessionListener::OnUsePreferredAddress(
int family,
const PreferredAddress& preferred_address) {
Expand Down Expand Up @@ -2377,7 +2356,10 @@ void QuicSession::SilentClose() {
err.code,
is_stateless_reset() ? "yes" : "no");

listener()->OnSessionSilentClose(is_stateless_reset(), err);
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
Expand Down
18 changes: 12 additions & 6 deletions src/quic/node_quic_session.h
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,12 @@ struct QuicSessionStatsTraits {

class QuicSessionListener {
public:
enum SessionCloseFlags {
SESSION_CLOSE_FLAG_NONE,
SESSION_CLOSE_FLAG_SILENT,
SESSION_CLOSE_FLAG_STATELESS_RESET
};

virtual ~QuicSessionListener();

virtual void OnKeylog(const char* str, size_t size);
Expand All @@ -280,7 +286,9 @@ class QuicSessionListener {
int64_t stream_id,
uint64_t app_error_code);
virtual void OnSessionDestroyed();
virtual void OnSessionClose(QuicError error);
virtual void OnSessionClose(
QuicError error,
int flags = SESSION_CLOSE_FLAG_NONE);
virtual void OnStreamReady(BaseObjectPtr<QuicStream> stream);
virtual void OnHandshakeCompleted();
virtual void OnPathValidation(
Expand All @@ -291,9 +299,6 @@ class QuicSessionListener {
int family,
const PreferredAddress& preferred_address);
virtual void OnSessionTicket(int size, SSL_SESSION* session);
virtual void OnSessionSilentClose(
bool stateless_reset,
QuicError error);
virtual void OnStreamBlocked(int64_t stream_id);
virtual void OnVersionNegotiation(
uint32_t supported_version,
Expand Down Expand Up @@ -329,15 +334,16 @@ class JSQuicSessionListener : public QuicSessionListener {
int64_t stream_id,
uint64_t app_error_code) override;
void OnSessionDestroyed() override;
void OnSessionClose(QuicError error) override;
void OnSessionClose(
QuicError error,
int flags = SESSION_CLOSE_FLAG_NONE) override;
void OnStreamReady(BaseObjectPtr<QuicStream> stream) override;
void OnHandshakeCompleted() override;
void OnPathValidation(
ngtcp2_path_validation_result res,
const sockaddr* local,
const sockaddr* remote) override;
void OnSessionTicket(int size, SSL_SESSION* session) override;
void OnSessionSilentClose(bool stateless_reset, QuicError error) override;
void OnUsePreferredAddress(
int family,
const PreferredAddress& preferred_address) override;
Expand Down