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

Signed-off-by: Tim Perry <pimterry@gmail.com>

Author: pimterry

src/quic/bindingdata.cc

@@ -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() {

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<BindingData, ngtcp2_mem> {
  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);

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);
+  }));
+}