Skip to content
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.

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

quic: more quic refactorings and cleanups #34283

Closed
wants to merge 18 commits into from
Closed
Changes from 1 commit
Commits
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: fixup kEndpointClose
Ensure that the QuicSocket is properly destroyed if the QuicEndpoint
is destroyed directly rather than through QuicSocket destroy
  • Loading branch information
jasnell committed Jul 16, 2020
commit b3a51d4a1595b87d7c0dc97ce03dd86ea6ccdc06
37 changes: 19 additions & 18 deletions lib/internal/quic/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -1052,7 +1052,6 @@ class QuicSocket extends EventEmitter {
});
}

// Called when a QuicEndpoint closes
[kEndpointClose](endpoint, error) {
const state = this[kInternalState];
state.endpoints.delete(endpoint);
Expand All @@ -1064,26 +1063,15 @@ class QuicSocket extends EventEmitter {
}
});

// If there are no more QuicEndpoints, the QuicSocket is no
// longer usable.
// If there aren't any more endpoints, the QuicSession
// is no longer usable and needs to be destroyed.
if (state.endpoints.size === 0) {
for (const session of state.sessions)
session.destroy(error);

if (error) process.nextTick(emit.bind(this, 'error', error));
process.nextTick(emit.bind(this, 'close'));
if (!this.destroyed)
return this.destroy(error);
this[kDestroy](error);
}
}

// kDestroy is called to actually free the QuicSocket resources and
// cause the error and close events to be emitted.
[kDestroy](error) {
// The QuicSocket will be destroyed once all QuicEndpoints
// are destroyed. See [kEndpointClose].
for (const endpoint of this[kInternalState].endpoints)
endpoint.destroy(error);
}

// kMaybeDestroy is called one or more times after the close() method
// is called. The QuicSocket will be destroyed if there are no remaining
// open sessions.
Expand Down Expand Up @@ -1463,7 +1451,20 @@ class QuicSocket extends EventEmitter {
for (const session of state.sessions)
session.destroy(error);

this[kDestroy](error);
// If there aren't any QuicEndpoints to clean up, skip
// directly to the end to emit the error and close events.
if (state.endpoints.size === 0)
return this[kDestroy](error);

// Otherwise, the QuicSocket will be destroyed once all
// QuicEndpoints are destroyed. See [kEndpointClose].
for (const endpoint of state.endpoints)
endpoint.destroy(error);
}

[kDestroy](error) {
if (error) process.nextTick(emit.bind(this, 'error', error));
process.nextTick(emit.bind(this, 'close'));
}

ref() {
Expand Down