quic: fix no onstream handler crash quic

Signed-off-by: Efe Karasakal <hi@efe.dev>
PR-URL: #64158
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>

Author: efekrskl

src/quic/application.cc

@@ -761,6 +761,12 @@ class DefaultApplication final : public Session::Application {
         // during the onstream callback (via MakeCallback re-entrancy).
         return false;
       }
+
+      // The stream was created, but was immediately destroyed because there's
+      // no onstream handler.
+      if (stream->is_destroyed()) [[unlikely]] {
+        return true;
+      }
     } else {
       stream = BaseObjectPtr<Stream>(Stream::From(stream_user_data));
       if (!stream) {

src/quic/session.cc

@@ -860,11 +860,11 @@ struct Session::Impl final : public MemoryRetainer {
       }
     }
 
-    endpoint->RemoveSession(config_.scid, remote_address_);
-
     auto& binding = BindingData::Get(env());
     if (stats_slot_) GetSessionStatsArena(binding).ReleaseSlot(stats_slot_);
     if (state_slot_) GetSessionStateArena(binding).ReleaseSlot(state_slot_);
+
+    endpoint->RemoveSession(config_.scid, remote_address_);
   }
 
   void MemoryInfo(MemoryTracker* tracker) const override {

src/quic/streams.cc

@@ -1325,6 +1325,10 @@ bool Stream::is_pending() const {
   return state()->pending;
 }
 
+bool Stream::is_destroyed() const {
+  return stats()->destroyed_at != 0;
+}
+
 stream_id Stream::id() const {
   return state()->id;
 }

src/quic/streams.h

@@ -267,6 +267,9 @@ class Stream final : public AsyncWrap,
   // to be created.
   bool is_pending() const;
 
+  // True if the stream is already destroyed.
+  bool is_destroyed() const;
+
   // True if we've completely sent all outbound data for this stream.
   // Importantly, this does not necessarily mean that we are completely
   // done with the outbound data. We may still be waiting on outbound

test/parallel/test-quic-stream-uni-no-onstream.mjs

@@ -0,0 +1,47 @@
+// Flags: --experimental-quic --no-warnings
+// Test: client-initiated unidirectional stream with no onstream
+// The client creates a uni stream with no onstream.
+// Verify that this causes no crash.
+
+import { hasQuic, skip, mustCall } from '../common/index.mjs';
+import * as fixtures from '../common/fixtures.mjs';
+
+const { readKey } = fixtures;
+
+if (!hasQuic) {
+  skip('QUIC is not enabled');
+}
+
+const { createPrivateKey } = await import('node:crypto');
+const { listen, connect } = await import('../common/quic.mjs');
+
+const serverKey = createPrivateKey(readKey('agent1-key.pem'));
+const serverCert = readKey('agent1-cert.pem');
+
+const done = Promise.withResolvers();
+
+const serverEndpoint = await listen(mustCall(async (session) => {
+  await session.opened;
+  done.resolve();
+}), {
+  sni: { '*': { keys: [serverKey], certs: [serverCert] } },
+  alpn: ['repro'],
+});
+
+const clientSession = await connect(serverEndpoint.address, {
+  servername: 'localhost',
+  alpn: 'repro',
+  ca: serverCert,
+});
+
+await clientSession.opened;
+
+await clientSession.createUnidirectionalStream({
+  body: 'something something darkside',
+});
+
+await done.promise;
+
+await clientSession.close();
+
+await serverEndpoint.close();