From e3bb3589e70f217742c493eb80bcb01458c135d1 Mon Sep 17 00:00:00 2001 From: Tim Perry Date: Thu, 17 Sep 2026 16:45:03 +0200 Subject: [PATCH] quic: cleanly tear down QUIC after worker.terminate() Signed-off-by: Tim Perry --- src/quic/bindingdata.cc | 8 ++++ src/quic/bindingdata.h | 4 ++ test/parallel/test-quic-worker-terminate.mjs | 46 ++++++++++++++++++++ 3 files changed, 58 insertions(+) create mode 100644 test/parallel/test-quic-worker-terminate.mjs diff --git a/src/quic/bindingdata.cc b/src/quic/bindingdata.cc index d5711d88cb92..e73ae82e980d 100644 --- a/src/quic/bindingdata.cc +++ b/src/quic/bindingdata.cc @@ -353,6 +353,14 @@ BindingData::BindingData(Realm* realm, Local 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() { diff --git a/src/quic/bindingdata.h b/src/quic/bindingdata.h index 96ac6a349182..2ef9f7685314 100644 --- a/src/quic/bindingdata.h +++ b/src/quic/bindingdata.h @@ -261,6 +261,7 @@ class CheckWrapHandle : public MemoryRetainer { // TODO(@jasnell): Make this snapshotable? class BindingData final : public BaseObject, + public Cleanable, public mem::NgLibMemoryManager { public: SET_BINDING_ID(quic_binding_data) @@ -392,6 +393,9 @@ class BindingData final bool flush_check_started_ = false; void OnFlushCheck(); + + private: + void Clean() override; }; JS_METHOD_IMPL(IllegalConstructor); diff --git a/test/parallel/test-quic-worker-terminate.mjs b/test/parallel/test-quic-worker-terminate.mjs new file mode 100644 index 000000000000..edd7b2186c71 --- /dev/null +++ b/test/parallel/test-quic-worker-terminate.mjs @@ -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); + })); +}