quic: consolidate onSessionClose and onSessionSilentClose

jasnell · jasnell · commit 3bae2d50733e · 2020-06-30T17:50:50.000-07:00
Use a single callback function for both PR-URL: nodejs#34137 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: David Carlier <devnexen@gmail.com>
diff --git a/lib/internal/quic/core.js b/lib/internal/quic/core.js
@@ -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.
@@ -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() {
@@ -581,7 +574,6 @@ setCallbacks({
   onSessionHandshake,
   onSessionKeylog,
   onSessionQlog,
-  onSessionSilentClose,
   onSessionStatus,
   onSessionTicket,
   onSessionVersionNegotiation,
diff --git a/src/env.h b/src/env.h
@@ -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)                \
diff --git a/src/quic/node_quic.cc b/src/quic/node_quic.cc
@@ -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);
diff --git a/src/quic/node_quic_session.cc b/src/quic/node_quic_session.cc
@@ -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) {
@@ -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) {
@@ -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
@@ -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) {
@@ -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
diff --git a/src/quic/node_quic_session.h b/src/quic/node_quic_session.h
@@ -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);
@@ -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(
@@ -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,
@@ -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;
