Skip to content

Commit

Permalink
quic: cleanup some outstanding todo items
Browse files Browse the repository at this point in the history
PR-URL: #34618
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
jasnell committed Aug 10, 2020
1 parent b0e4970 commit 6b0b33c
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 29 deletions.
44 changes: 17 additions & 27 deletions lib/internal/quic/core.js
Expand Up @@ -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');
Expand Down Expand Up @@ -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();
}
}
Expand Down Expand Up @@ -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) {
Expand All @@ -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
Expand Down Expand Up @@ -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) ?
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -2910,11 +2908,6 @@ class QuicStream extends Duplex {
undefined;
}

get bufferSize() {
// TODO(@jasnell): Implement this
return undefined;
}

get id() {
return this[kInternalState].id;
}
Expand All @@ -2923,10 +2916,6 @@ class QuicStream extends Duplex {
return this[kInternalState].push_id;
}

_onTimeout() {
// TODO(@jasnell): Implement this
}

get session() {
return this[kInternalState].session;
}
Expand Down Expand Up @@ -3127,7 +3116,8 @@ function createSocket(options) {

module.exports = {
createSocket,
kUDPHandleForTesting
kUDPHandleForTesting,
kRemoveFromSocket,
};

/* eslint-enable no-use-before-define */
Expand Down
5 changes: 3 additions & 2 deletions 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)
Expand All @@ -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');

Expand Down Expand Up @@ -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
}));

Expand Down

0 comments on commit 6b0b33c

Please sign in to comment.