Skip to content

Commit 3c9eeb3

Browse files
committed
quic: release stream arenas before cleanup
Release QUIC stream stats and state arena slots when the stream is destroyed instead of from the Stream destructor. Realm cleanup destroys QUIC binding data before draining remaining BaseObjects, so a stream that survives until process teardown must not need BindingData from its destructor. Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com> Assisted-by: codex:gpt-5.6-sol
1 parent 46541f9 commit 3c9eeb3

3 files changed

Lines changed: 42 additions & 5 deletions

File tree

‎src/quic/streams.cc‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1211,11 +1211,9 @@ Stream::Stream(BaseObjectWeakPtr<Session> session,
12111211
STAT_SET(Stats, max_offset, params->initial_max_data);
12121212
}
12131213

1214-
Stream::~Stream() {
1215-
// Make sure that Destroy() was called before Stream is actually destructed.
1216-
DCHECK_NE(stats()->destroyed_at, 0);
1214+
Stream::~Stream() = default;
12171215

1218-
// Release arena slots back to the freelist.
1216+
void Stream::ReleaseArenaSlots() {
12191217
auto& binding = BindingData::Get(env());
12201218
if (stats_slot_) {
12211219
GetStreamStatsArena(binding).ReleaseSlot(stats_slot_);
@@ -1306,6 +1304,7 @@ bool Stream::is_pending() const {
13061304
}
13071305

13081306
bool Stream::is_destroyed() const {
1307+
if (!stats_slot_) return true;
13091308
return stats()->destroyed_at != 0;
13101309
}
13111310

@@ -1666,14 +1665,17 @@ void Stream::Destroy(QuicError error) {
16661665
// handle.
16671666
EmitClose(error);
16681667

1668+
stream_id id_to_remove = id();
1669+
ReleaseArenaSlots();
1670+
16691671
auto session = session_;
16701672
session_.reset();
16711673
// EmitClose above triggers MakeCallback which can destroy the session
16721674
// via JS re-entrancy. The weak pointer may still be non-null (the
16731675
// Session BaseObject can be kept alive by a BaseObjectPtr elsewhere,
16741676
// e.g. OnTimeout's ref) even though impl_ has been reset. We must
16751677
// check is_destroyed() to avoid dereferencing the null impl_.
1676-
if (session && !session->is_destroyed()) session->RemoveStream(id());
1678+
if (session && !session->is_destroyed()) session->RemoveStream(id_to_remove);
16771679

16781680
// Critically, make sure that the RemoveStream call is the last thing
16791681
// trying to use this stream object. Once that call is made, the stream

‎src/quic/streams.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -408,6 +408,7 @@ class Stream final : public AsyncWrap,
408408

409409
bool is_local_unidirectional() const;
410410
bool is_remote_unidirectional() const;
411+
void ReleaseArenaSlots();
411412

412413
// JavaScript callouts
413414

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// Flags: --experimental-quic --no-warnings
2+
3+
// Regression test for https://github.com/nodejs/node/issues/65408.
4+
// A client-created unidirectional stream is not a valid HTTP/3 request stream,
5+
// but nghttp3 handles it internally. Destroying the endpoint after receiving
6+
// data on that stream must not crash during process teardown.
7+
8+
import { hasQuic, skip, mustNotCall } from '../common/index.mjs';
9+
import * as fixtures from '../common/fixtures.mjs';
10+
11+
if (!hasQuic) {
12+
skip('QUIC is not enabled');
13+
}
14+
15+
const { createPrivateKey } = await import('node:crypto');
16+
const { listen, connect } = await import('node:quic');
17+
18+
const key = createPrivateKey(fixtures.readKey('agent1-key.pem'));
19+
const cert = fixtures.readKey('agent1-cert.pem');
20+
21+
const endpoint = await listen(mustNotCall(), {
22+
sni: { '*': { keys: [key], certs: [cert] } },
23+
});
24+
25+
const session = await connect(endpoint.address, {
26+
servername: 'localhost',
27+
verifyPeer: 'manual',
28+
});
29+
30+
const stream = await session.createUnidirectionalStream();
31+
stream.writer.writeSync('x');
32+
33+
endpoint.destroy();
34+
await endpoint.closed;

0 commit comments

Comments
 (0)