Skip to content
This repository was archived by the owner on Aug 11, 2020. It is now read-only.

Commit 210ec62

Browse files
addaleaxjasnell
authored andcommitted
quic: test cleanup when session is detached from QuicSocket
Previously, the QuicSocket instance would always be destroyed first during cleanup (because it is a `HandleWrap`), destroying the other native objects associated with it in the process. This makes sure that cleanup also works when the session is detached from the “real” network socket. In particular, this is useful for the future possibility of fully detaching sessions from sockets. PR-URL: #141 Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 94b4995 commit 210ec62

File tree

3 files changed

+24
-3
lines changed

3 files changed

+24
-3
lines changed

lib/internal/quic/core.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1763,6 +1763,12 @@ class QuicSession extends EventEmitter {
17631763
get handshakeContinuationHistogram() {
17641764
return this.#handshakeContinuationHistogram;
17651765
}
1766+
1767+
// TODO(addaleax): This is a temporary solution for testing and should be
1768+
// removed later.
1769+
removeFromSocket() {
1770+
return this[kHandle].removeFromSocket();
1771+
}
17661772
}
17671773

17681774
class QuicServerSession extends QuicSession {

src/node_quic_session.cc

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1472,7 +1472,7 @@ bool QuicSession::SendPacket(const char* diagnostic_label) {
14721472
txbuf_ += std::move(sendbuf_);
14731473
}
14741474
// There's nothing to send, so let's not try
1475-
if (txbuf_.Length() == 0)
1475+
if (txbuf_.Length() == 0 || Socket() == nullptr)
14761476
return true;
14771477
Debug(this, "There are %" PRIu64 " bytes in txbuf_ to send", txbuf_.Length());
14781478
session_stats_.session_sent_at = uv_hrtime();
@@ -3577,6 +3577,14 @@ void QuicSessionPing(const FunctionCallbackInfo<Value>& args) {
35773577
session->Ping();
35783578
}
35793579

3580+
// TODO(addaleax): This is a temporary solution for testing and should be
3581+
// removed later.
3582+
void QuicSessionRemoveFromSocket(const FunctionCallbackInfo<Value>& args) {
3583+
QuicSession* session;
3584+
ASSIGN_OR_RETURN_UNWRAP(&session, args.Holder());
3585+
session->RemoveFromSocket();
3586+
}
3587+
35803588
void QuicSessionUpdateKey(const FunctionCallbackInfo<Value>& args) {
35813589
QuicSession* session;
35823590
ASSIGN_OR_RETURN_UNWRAP(&session, args.Holder());
@@ -3658,6 +3666,7 @@ void AddMethods(Environment* env, Local<FunctionTemplate> session) {
36583666
env->SetProtoMethod(session, "gracefulClose", QuicSessionGracefulClose);
36593667
env->SetProtoMethod(session, "updateKey", QuicSessionUpdateKey);
36603668
env->SetProtoMethod(session, "ping", QuicSessionPing);
3669+
env->SetProtoMethod(session, "removeFromSocket", QuicSessionRemoveFromSocket);
36613670
env->SetProtoMethod(session, "onClientHelloDone",
36623671
QuicSessionOnClientHelloDone);
36633672
env->SetProtoMethod(session, "onCertDone", QuicSessionOnCertDone);

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,13 @@ if (!common.hasQuic)
88
// sequence and we can stop execution at any point.
99

1010
const quic = require('quic');
11-
const { isMainThread, Worker } = require('worker_threads');
11+
const { isMainThread, Worker, workerData } = require('worker_threads');
1212

13-
if (isMainThread) return new Worker(__filename);
13+
if (isMainThread) {
14+
new Worker(__filename, { workerData: { removeFromSocket: true } });
15+
new Worker(__filename, { workerData: { removeFromSocket: false } });
16+
return;
17+
}
1418

1519
const fixtures = require('../common/fixtures');
1620
const key = fixtures.readKey('agent1-key.pem', 'binary');
@@ -58,6 +62,8 @@ server.on('ready', common.mustCall(() => {
5862
});
5963

6064
req.on('stream', common.mustCall(() => {
65+
if (workerData.removeFromSocket)
66+
req.removeFromSocket();
6167
process.exit();
6268
}));
6369

0 commit comments

Comments
 (0)