Skip to content

quic: cleanly tear down QUIC after worker.terminate() #66093

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

Open
pimterry wants to merge 1 commit into
base: main
Choose a base branch
from
+58 −0
Open

quic: cleanly tear down QUIC after worker.terminate() #66093

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 8 additions & 0 deletions src/quic/bindingdata.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,14 @@ BindingData::BindingData(Realm* realm, Local<Object> object)
MakeWeak();
// Unref so the check handle doesn't keep the event loop alive on its own.
flush_check_.Unref();
// Ensure Clean() below is called before the tearing anything down.
env()->cleanable_queue()->PushFront(this);
}

void BindingData::Clean() {
// Make sure sessions are always properly destroyed. This does nothing in
// a clean shutdown, but is required for cases like worker.terminate().
if (session_manager_) session_manager_->DestroyAllSessions();
}

SessionManager& BindingData::session_manager() {
Expand Down
4 changes: 4 additions & 0 deletions src/quic/bindingdata.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,7 @@ class CheckWrapHandle : public MemoryRetainer {
// TODO(@jasnell): Make this snapshotable?
class BindingData final
: public BaseObject,
public Cleanable,
public mem::NgLibMemoryManager<BindingData, ngtcp2_mem> {
public:
SET_BINDING_ID(quic_binding_data)
Expand Down Expand Up @@ -392,6 +393,9 @@ class BindingData final
bool flush_check_started_ = false;

void OnFlushCheck();

private:
void Clean() override;
};

JS_METHOD_IMPL(IllegalConstructor);
Expand Down
46 changes: 46 additions & 0 deletions test/parallel/test-quic-worker-terminate.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// Flags: --experimental-quic --no-warnings

// Test: terminating a worker thread that still holds live QUIC sessions.
//
// worker.terminate() tears the environment down without running any of the
// JavaScript close paths, so the sessions are still open when the QUIC
// binding is cleaned up. Sessions must be properly destroyed before reaching
// ~Session.

import { hasQuic, skip, mustCall } from '../common/index.mjs';
import assert from 'node:assert';
import { Worker, isMainThread, parentPort } from 'node:worker_threads';

if (!hasQuic) {
skip('QUIC is not enabled');
}

const { listen, connect } = await import('../common/quic.mjs');

// Launch a client and server in a worker thread, then kill it:
if (!isMainThread) {
// A client and a server session, both with an open stream, and neither
// closed. The worker then parks forever waiting to be terminated.
const serverEndpoint = await listen((session) => {
session.closed.catch(() => {});
session.onstream = (stream) => { stream.closed.catch(() => {}); };
});

const clientSession = await connect(serverEndpoint.address);
clientSession.closed.catch(() => {});
await clientSession.opened;
const stream = await clientSession.createBidirectionalStream({
body: new Uint8Array(1),
});
stream.closed.catch(() => {});

parentPort.postMessage('ready');
await new Promise(() => {});
} else {
const worker = new Worker(new URL(import.meta.url));
worker.on('error', (err) => { assert.fail(err); });
worker.on('message', mustCall(async (message) => {
assert.strictEqual(message, 'ready');
assert.strictEqual(await worker.terminate(), 1);
}));
}
Loading