Skip to content

quic: continued refactoring of quic APIs #34351

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.

Sign up for GitHub

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

Closed
jasnell wants to merge 8 commits into from
Closed

quic: continued refactoring of quic APIs #34351

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3aefa5e
quic: convert openStream to Promise
jasnell Jul 14, 2020
9999fa0
quic: simplify QuicStream construction logic
jasnell Jul 14, 2020
42ee750
quic: remove stream pending code
jasnell Jul 14, 2020
21a96cb
quic: remove unneeded quicstream.aborted and fixup docs
jasnell Jul 14, 2020
ec838a6
quic: gitignore qlog files
jasnell Jul 15, 2020
c398c63
quic: extensive refactoring of QuicStream lifecycle
jasnell Jul 14, 2020
53de1b2
quic: documentation updates
jasnell Jul 16, 2020
c3a33be
quic: use async _construct for QuicStream
jasnell Jul 22, 2020
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: remove stream pending code 
Removing no longer needed code
  • Loading branch information
@jasnell
jasnell committed Jul 23, 2020
commit 42ee750b72e831c011895623132fbb18f73ea658
43 changes: 1 addition & 42 deletions lib/internal/quic/core.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1666,7 +1666,6 @@ class QuicSession extends EventEmitter {
silentClose: false,
statelessReset: false,
stats: undefined,
pendingStreams: new Set(),
streams: new Map(),
verifyErrorReason: undefined,
verifyErrorCode: undefined,
Expand Down Expand Up @@ -1969,12 +1968,6 @@ class QuicSession extends EventEmitter {
return;
state.destroyed = true;

// Destroy any pending streams immediately. These
// are streams that have been created but have not
// yet been assigned an internal handle.
for (const stream of state.pendingStreams)
stream.destroy(error);

// Destroy any remaining streams immediately.
for (const stream of state.streams.values())
stream.destroy(error);
Expand Down Expand Up @@ -2534,7 +2527,7 @@ function streamOnResume() {
}

function streamOnPause() {
if (!this.destroyed /* && !this.pending */)
if (!this.destroyed)
this[kHandle].readStop();
}

Expand Down Expand Up @@ -2658,9 +2651,6 @@ class QuicStream extends Duplex {
if (this.destroyed || state.closed)
return;

if (this.pending)
return this.once('ready', () => this[kClose](family, code));

state.closed = true;

state.aborted = this.readable || this.writable;
Expand Down Expand Up @@ -2712,11 +2702,6 @@ class QuicStream extends Duplex {
// TODO(@jasnell): Implement this later
}

get pending() {
// The id is set in the kSetHandle function
return this[kInternalState].id === undefined;
}

get aborted() {
return this[kInternalState].aborted;
}
Expand All @@ -2741,16 +2726,6 @@ class QuicStream extends Duplex {
if (this.destroyed)
return; // TODO(addaleax): Can this happen?

// The stream should be corked while still pending
// but just in case uncork
// was called early, defer the actual write until the
// ready event is emitted.
if (this.pending) {
return this.once('ready', () => {
this[kWriteGeneric](writev, data, encoding, cb);
});
}

this[kUpdateTimer]();
const req = (writev) ?
writevGeneric(this, data, cb) :
Expand All @@ -2774,13 +2749,6 @@ class QuicStream extends Duplex {
// coming so that a fin stream packet can be
// sent.
_final(cb) {
// The QuicStream should be corked while pending
// so this shouldn't be called, but just in case
// the stream was prematurely uncorked, defer the
// operation until the ready event is emitted.
if (this.pending)
return this.once('ready', () => this._final(cb));

const handle = this[kHandle];
if (handle === undefined) {
cb();
Expand All @@ -2796,9 +2764,6 @@ class QuicStream extends Duplex {
}

_read(nread) {
if (this.pending)
return this.once('ready', () => this._read(nread));

if (this.destroyed) { // TODO(addaleax): Can this happen?
this.push(null);
return;
Expand Down Expand Up @@ -2848,12 +2813,6 @@ class QuicStream extends Duplex {
else if (typeof fd !== 'number')
throw new ERR_INVALID_ARG_TYPE('fd', ['number', 'FileHandle'], fd);

if (this.pending) {
return this.once('ready', () => {
this.sendFD(fd, { offset, length }, ownsFd);
});
}

this[kUpdateTimer]();
this.ownsFd = ownsFd;

Expand Down
1 change: 0 additions & 1 deletion test/parallel/test-quic-client-server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,6 @@ client.on('close', common.mustCall(onSocketClose.bind(client)));
assert(!stream.unidirectional);
assert(stream.clientInitiated);
assert(!stream.serverInitiated);
assert(!stream.pending);

const file = fs.createReadStream(__filename);
let data = '';
Expand Down