quic: release stream arenas before cleanup

src/quic/http3.cc

@@ -849,6 +849,10 @@ class Http3ApplicationImpl final : public Session::Application {
           "HTTP/3 application received end of headers for stream %" PRIi64,
           id);
     stream->EmitHeaders();
+    // EmitHeaders() calls into JavaScript, which can synchronously destroy the
+    // stream. Its arena-backed state is released by Destroy(), so do not touch
+    // the stream again if that happened.
+    if (stream->is_destroyed()) return;
     if (fin) {
       // The stream is done. There's no more data to receive!
       Debug(&session(), "Headers are final for stream %" PRIi64, id);
@@ -891,6 +895,10 @@ class Http3ApplicationImpl final : public Session::Application {
           "HTTP/3 application received end of trailers for stream %" PRIi64,
           id);
     stream->EmitHeaders();
+    // EmitHeaders() calls into JavaScript, which can synchronously destroy the
+    // stream. Its arena-backed state is released by Destroy(), so do not touch
+    // the stream again if that happened.
+    if (stream->is_destroyed()) return;
     if (fin) {
       Debug(&session(), "Trailers are final for stream %" PRIi64, id);
       Stream::ReceiveDataFlags flags{

src/quic/streams.cc

@@ -1211,11 +1211,9 @@ Stream::Stream(BaseObjectWeakPtr<Session> session,
   STAT_SET(Stats, max_offset, params->initial_max_data);
 }
 
-Stream::~Stream() {
-  // Make sure that Destroy() was called before Stream is actually destructed.
-  DCHECK_NE(stats()->destroyed_at, 0);
+Stream::~Stream() = default;
 
-  // Release arena slots back to the freelist.
+void Stream::ReleaseArenaSlots() {
   auto& binding = BindingData::Get(env());
   if (stats_slot_) {
     GetStreamStatsArena(binding).ReleaseSlot(stats_slot_);
@@ -1306,6 +1304,7 @@ bool Stream::is_pending() const {
 }
 
 bool Stream::is_destroyed() const {
+  if (!stats_slot_) return true;
   return stats()->destroyed_at != 0;
 }
 
@@ -1621,7 +1620,7 @@ void Stream::EndReadable(std::optional<uint64_t> maybe_final_size) {
 }
 
 void Stream::Destroy(QuicError error) {
-  if (stats()->destroyed_at != 0) return;
+  if (is_destroyed()) return;
 
   // Record the destroyed at timestamp before notifying the JavaScript side
   // that the stream is being destroyed.
@@ -1666,14 +1665,17 @@ void Stream::Destroy(QuicError error) {
   // handle.
   EmitClose(error);
 
+  stream_id id_to_remove = id();
+  ReleaseArenaSlots();
+
   auto session = session_;
   session_.reset();
   // EmitClose above triggers MakeCallback which can destroy the session
   // via JS re-entrancy. The weak pointer may still be non-null (the
   // Session BaseObject can be kept alive by a BaseObjectPtr elsewhere,
   // e.g. OnTimeout's ref) even though impl_ has been reset. We must
   // check is_destroyed() to avoid dereferencing the null impl_.
-  if (session && !session->is_destroyed()) session->RemoveStream(id());
+  if (session && !session->is_destroyed()) session->RemoveStream(id_to_remove);
 
   // Critically, make sure that the RemoveStream call is the last thing
   // trying to use this stream object. Once that call is made, the stream

src/quic/streams.h

@@ -408,6 +408,7 @@ class Stream final : public AsyncWrap,
 
   bool is_local_unidirectional() const;
   bool is_remote_unidirectional() const;
+  void ReleaseArenaSlots();
 
   // JavaScript callouts
 

test/parallel/test-quic-h3-uni-stream-teardown.mjs

@@ -0,0 +1,34 @@
+// Flags: --experimental-quic --no-warnings
+
+// Regression test for https://github.com/nodejs/node/issues/65408.
+// A client-created unidirectional stream is not a valid HTTP/3 request stream,
+// but nghttp3 handles it internally. Destroying the endpoint after receiving
+// data on that stream must not crash during process teardown.
+
+import { hasQuic, skip, mustNotCall } from '../common/index.mjs';
+import * as fixtures from '../common/fixtures.mjs';
+
+if (!hasQuic) {
+  skip('QUIC is not enabled');
+}
+
+const { createPrivateKey } = await import('node:crypto');
+const { listen, connect } = await import('node:quic');
+
+const key = createPrivateKey(fixtures.readKey('agent1-key.pem'));
+const cert = fixtures.readKey('agent1-cert.pem');
+
+const endpoint = await listen(mustNotCall(), {
+  sni: { '*': { keys: [key], certs: [cert] } },
+});
+
+const session = await connect(endpoint.address, {
+  servername: 'localhost',
+  verifyPeer: 'manual',
+});
+
+const stream = await session.createUnidirectionalStream();
+stream.writer.writeSync('x');
+
+endpoint.destroy();
+await endpoint.closed;