quic: cleanup some outstanding todo items

PR-URL: #34618
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>

Author: jasnell

lib/internal/quic/core.js

@@ -225,6 +225,7 @@ const kOnFileOpened = Symbol('kOnFileOpened');
 const kOnFileUnpipe = Symbol('kOnFileUnpipe');
 const kOnPipedFileHandleRead = Symbol('kOnPipedFileHandleRead');
 const kReady = Symbol('kReady');
+const kRemoveFromSocket = Symbol('kRemoveFromSocket');
 const kRemoveSession = Symbol('kRemove');
 const kRemoveStream = Symbol('kRemoveStream');
 const kServerBusy = Symbol('kServerBusy');
@@ -2247,9 +2248,7 @@ class QuicSession extends EventEmitter {
     return this[kInternalState].handshakeContinuationHistogram;
   }
 
-  // TODO(addaleax): This is a temporary solution for testing and should be
-  // removed later.
-  removeFromSocket() {
+  [kRemoveFromSocket]() {
     return this[kHandle].removeFromSocket();
   }
 }
@@ -2695,7 +2694,17 @@ class QuicStream extends Duplex {
   }
 
   [kAfterAsyncWrite]({ bytes }) {
-    // TODO(@jasnell): Implement this
+    // There's currently nothing we need to do here. We have
+    // to have this but it's a non-op
+  }
+
+  [kUpdateTimer]() {
+    // Timeout is implemented in the QuicSession at the native
+    // layer. We have to have this here but it's a non-op
+  }
+
+  [kTrackWriteState](stream, bytes) {
+    // There's currently nothing to do here.
   }
 
   [kInspect](depth, options) {
@@ -2712,17 +2721,6 @@ class QuicStream extends Duplex {
     }, depth, options);
   }
 
-  [kTrackWriteState](stream, bytes) {
-    // TODO(@jasnell): Not yet sure what we want to do with these
-    // this.#writeQueueSize += bytes;
-    // this.#writeQueueSize += bytes;
-    // this[kHandle].chunksSentSinceLastWrite = 0;
-  }
-
-  [kUpdateTimer]() {
-    // TODO(@jasnell): Implement this later
-  }
-
   get detached() {
     // The QuicStream is detached if it is yet destroyed
     // but the underlying handle is undefined. While in
@@ -2750,7 +2748,7 @@ class QuicStream extends Duplex {
 
   [kWriteGeneric](writev, data, encoding, cb) {
     if (this.destroyed || this.detached)
-      return;  // TODO(addaleax): Can this happen?
+      return;
 
     this[kUpdateTimer]();
     const req = (writev) ?
@@ -2810,7 +2808,7 @@ class QuicStream extends Duplex {
   }
 
   _read(nread) {
-    if (this.destroyed) {  // TODO(addaleax): Can this happen?
+    if (this.destroyed) {
       this.push(null);
       return;
     }
@@ -2910,11 +2908,6 @@ class QuicStream extends Duplex {
       undefined;
   }
 
-  get bufferSize() {
-    // TODO(@jasnell): Implement this
-    return undefined;
-  }
-
   get id() {
     return this[kInternalState].id;
   }
@@ -2923,10 +2916,6 @@ class QuicStream extends Duplex {
     return this[kInternalState].push_id;
   }
 
-  _onTimeout() {
-    // TODO(@jasnell): Implement this
-  }
-
   get session() {
     return this[kInternalState].session;
   }
@@ -3127,7 +3116,8 @@ function createSocket(options) {
 
 module.exports = {
   createSocket,
-  kUDPHandleForTesting
+  kUDPHandleForTesting,
+  kRemoveFromSocket,
 };
 
 /* eslint-enable no-use-before-define */

test/parallel/test-quic-process-cleanup.js

@@ -1,4 +1,4 @@
-// Flags: --no-warnings
+// Flags: --no-warnings --expose-internals
 'use strict';
 const common = require('../common');
 if (!common.hasQuic)
@@ -8,6 +8,7 @@ if (!common.hasQuic)
 // well. We use Workers because they have a more clearly defined shutdown
 // sequence and we can stop execution at any point.
 
+const { kRemoveFromSocket } = require('internal/quic/core');
 const { createQuicSocket } = require('net');
 const { Worker, workerData } = require('worker_threads');
 
@@ -46,7 +47,7 @@ client.on('close', common.mustNotCall());
 
   req.on('stream', common.mustCall(() => {
     if (workerData.removeFromSocket)
-      req.removeFromSocket();
+      req[kRemoveFromSocket]();
     process.exit();  // Exits the worker thread
   }));