quic: refactor QuicSession close/destroy flow

PR-URL: #34160
Reviewed-By: Anna Henningsen <anna@addaleax.net>

Author: jasnell

lib/internal/quic/core.js

@@ -287,24 +287,10 @@ function onSessionReady(handle) {
   process.nextTick(emit.bind(socket, 'session', session));
 }
 
-// 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.
+// Called when the C++ QuicSession::Close() method has been called.
+// Synchronously cleanup and destroy the JavaScript QuicSession.
 function onSessionClose(code, family, silent, statelessReset) {
-  if (this[owner_symbol]) {
-    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);
+  this[owner_symbol][kDestroy](code, family, silent, statelessReset);
 }
 
 // Called by the C++ internals when a QuicSession has been destroyed.
@@ -1654,6 +1640,7 @@ class QuicSession extends EventEmitter {
     maxPacketLength: NGTCP2_DEFAULT_MAX_PKTLEN,
     servername: undefined,
     socket: undefined,
+    silentClose: false,
     statelessReset: false,
     stats: undefined,
     pendingStreams: new Set(),
@@ -1736,46 +1723,15 @@ class QuicSession extends EventEmitter {
 
   // Causes the QuicSession to be immediately destroyed, but with
   // additional metadata set.
-  [kDestroy](statelessReset, family, code) {
+  [kDestroy](code, family, silent, statelessReset) {
     const state = this[kInternalState];
-    state.statelessReset = statelessReset;
     state.closeCode = code;
     state.closeFamily = family;
+    state.silentClose = silent;
+    state.statelessReset = statelessReset;
     this.destroy();
   }
 
-  // Immediate close has been initiated for the session. Any
-  // still open QuicStreams must be abandoned and shutdown
-  // with RESET_STREAM and STOP_SENDING frames transmitted
-  // as appropriate. Once the streams have been shutdown, a
-  // CONNECTION_CLOSE will be generated and sent, switching
-  // the session into the closing period.
-  [kClose](family, code) {
-    const state = this[kInternalState];
-    // Do nothing if the QuicSession has already been destroyed.
-    if (state.destroyed)
-      return;
-
-    // Set the close code and family so we can keep track.
-    state.closeCode = code;
-    state.closeFamily = family;
-
-    // Shutdown all pending streams. These are Streams that
-    // have been created but do not yet have a handle assigned.
-    for (const stream of state.pendingStreams)
-      stream[kClose](family, code);
-
-    // Shutdown all of the remaining streams
-    for (const stream of state.streams.values())
-      stream[kClose](family, code);
-
-    // By this point, all necessary RESET_STREAM and
-    // STOP_SENDING frames ought to have been sent,
-    // so now we just trigger sending of the
-    // CONNECTION_CLOSE frame.
-    this[kHandle].close(family, code);
-  }
-
   // Closes the specified stream with the given code. The
   // QuicStream object will be destroyed.
   [kStreamClose](id, code) {
@@ -1873,14 +1829,6 @@ class QuicSession extends EventEmitter {
     this[kInternalState].streams.set(id, stream);
   }
 
-  // The QuicSession will be destroyed if closing has been
-  // called and there are no remaining streams
-  [kMaybeDestroy]() {
-    const state = this[kInternalState];
-    if (state.closing && state.streams.size === 0)
-      this.destroy();
-  }
-
   // Called when a client QuicSession has opted to use the
   // server provided preferred address. This is a purely
   // informationational notification. It is not called on
@@ -1890,6 +1838,17 @@ class QuicSession extends EventEmitter {
       emit.bind(this, 'usePreferredAddress', address));
   }
 
+  // The QuicSession will be destroyed if close() has been
+  // called and there are no remaining streams
+  [kMaybeDestroy]() {
+    const state = this[kInternalState];
+    if (state.closing && state.streams.size === 0) {
+      this.destroy();
+      return true;
+    }
+    return false;
+  }
+
   // Closing allows any existing QuicStream's to complete
   // normally but disallows any new QuicStreams from being
   // opened. Calls to openStream() will fail, and new streams
@@ -1910,27 +1869,27 @@ class QuicSession extends EventEmitter {
     // has been destroyed
     if (state.closing)
       return;
-
     state.closing = true;
-    this[kHandle].gracefulClose();
 
-    // See if we can close immediately.
-    this[kMaybeDestroy]();
+    // If there are no active streams, we can close immediately,
+    // otherwise set the graceful close flag.
+    if (!this[kMaybeDestroy]())
+      this[kHandle].gracefulClose();
   }
 
   // Destroying synchronously shuts down and frees the
   // QuicSession immediately, even if there are still open
   // streams.
   //
-  // A CONNECTION_CLOSE packet is sent to the
-  // connected peer and the session is immediately
-  // destroyed.
+  // Unless we're in the middle of a silent close, a
+  // CONNECTION_CLOSE packet will be sent to the connected
+  // peer and the session will be immediately destroyed.
   //
   // If destroy is called with an error argument, the
   // 'error' event is emitted on next tick.
   //
   // Once destroyed, and after the 'error' event (if any),
-  // the close event is emitted on next tick.
+  // the 'close' event is emitted on next tick.
   destroy(error) {
     const state = this[kInternalState];
     // Destroy can only be called once. Multiple calls will be ignored
@@ -1939,19 +1898,6 @@ class QuicSession extends EventEmitter {
     state.destroyed = true;
     state.closing = false;
 
-    if (typeof error === 'number' ||
-        (error != null &&
-         typeof error === 'object' &&
-         !(error instanceof Error))) {
-      const {
-        closeCode,
-        closeFamily
-      } = validateCloseCode(error);
-      state.closeCode = closeCode;
-      state.closeFamily = closeFamily;
-      error = new ERR_QUIC_ERROR(closeCode, closeFamily);
-    }
-
     // Destroy any pending streams immediately. These
     // are streams that have been created but have not
     // yet been assigned an internal handle.
@@ -1965,16 +1911,20 @@ class QuicSession extends EventEmitter {
     this.removeListener('newListener', onNewListener);
     this.removeListener('removeListener', onRemoveListener);
 
+    // If we are destroying with an error, schedule the
+    // error to be emitted on process.nextTick.
     if (error) process.nextTick(emit.bind(this, 'error', error));
 
     const handle = this[kHandle];
+    this[kHandle] = undefined;
+
     if (handle !== undefined) {
       // Copy the stats for use after destruction
       state.stats = new BigInt64Array(handle.stats);
-      state.idleTimeout = !!handle.state[IDX_QUIC_SESSION_STATE_IDLE_TIMEOUT];
-      // Calling destroy will cause a CONNECTION_CLOSE to be
-      // sent to the peer and will destroy the QuicSession
-      // handler immediately.
+      state.idleTimeout =
+        Boolean(handle.state[IDX_QUIC_SESSION_STATE_IDLE_TIMEOUT]);
+
+      // Destroy the underlying QuicSession handle
       handle.destroy(state.closeCode, state.closeFamily);
     } else {
       process.nextTick(emit.bind(this, 'close'));
@@ -2108,7 +2058,8 @@ class QuicSession extends EventEmitter {
     const state = this[kInternalState];
     return {
       code: state.closeCode,
-      family: state.closeFamily
+      family: state.closeFamily,
+      silent: state.silentClose,
     };
   }
 

src/quic/node_quic_session-inl.h

@@ -348,7 +348,7 @@ void QuicSession::OnIdleTimeout() {
   if (!is_destroyed()) {
     state_[IDX_QUIC_SESSION_STATE_IDLE_TIMEOUT] = 1;
     Debug(this, "Idle timeout");
-    SilentClose();
+    Close(QuicSessionListener::SESSION_CLOSE_FLAG_SILENT);
   }
 }