Skip to content

Commit 975cac5

Browse files
martenrichteraduh95
authored andcommitted
quic: add promise to QuicStream for pending strms
before this PR, it was necessary to poll, if a stream can not be created immediately due to flow control. This PR adds a promise to QuicStream, that fulfills, when a stream is available and ready. Signed-off-by: Marten Richter <marten.richter@freenet.de> PR-URL: #65862 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tim Perry <pimterry@gmail.com>
1 parent e51673b commit 975cac5

10 files changed

Lines changed: 109 additions & 1 deletion

File tree

doc/api/quic.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1861,6 +1861,19 @@ Either `'application'` or `'transport'`. Indicates the namespace of
18611861
added: v23.8.0
18621862
-->
18631863

1864+
### `stream.opened`
1865+
1866+
<!-- YAML
1867+
added: REPLACEME
1868+
-->
1869+
1870+
* Type: {Promise}
1871+
1872+
A promise that is immediately fulfilled, if the stream fits within
1873+
flow control limits or fulfilled when the pending stream is created.
1874+
It rejects, if a pending stream is closed with an error before being
1875+
created.
1876+
18641877
### `stream.closed`
18651878

18661879
<!-- YAML

lib/internal/quic/quic.js

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,7 @@ const kEmptyObject = { __proto__: null };
184184

185185
const {
186186
kAttachFileHandle,
187+
kAvailable,
187188
kBlocked,
188189
kConnect,
189190
kDatagram,
@@ -948,6 +949,11 @@ setCallbacks({
948949
},
949950

950951
// QuicStream callbacks
952+
onStreamAvailable() {
953+
debug('stream available callback', this[kOwner]);
954+
this[kOwner][kAvailable]();
955+
},
956+
951957
onStreamBlocked() {
952958
debug('stream blocked callback', this[kOwner]);
953959
// Called when the stream C++ handle has been blocked by flow control.
@@ -1602,6 +1608,7 @@ class QuicStream {
16021608
fileHandle: undefined,
16031609
headers: undefined,
16041610
pendingTrailers: undefined,
1611+
pendingStream: PromiseWithResolvers(),
16051612
onerror: undefined,
16061613
onblocked: undefined,
16071614
onreset: undefined,
@@ -1655,6 +1662,10 @@ class QuicStream {
16551662
inner.state = new QuicStreamState(
16561663
kPrivateConstructor, handle.state, handle.stateByteOffset);
16571664

1665+
if (!inner.state.pending) {
1666+
inner.pendingStream.resolve();
1667+
}
1668+
16581669
if (hasObserver('quic')) {
16591670
startPerf(this, kPerfEntry, { type: 'quic', name: 'QuicStream' });
16601671
}
@@ -1719,6 +1730,15 @@ class QuicStream {
17191730
return this.#inner.state.pending;
17201731
}
17211732

1733+
/**
1734+
* Promise that resolves once the stream is available and not pending.
1735+
* @type {Promise<void>}
1736+
*/
1737+
get opened() {
1738+
assertIsQuicStream(this);
1739+
return this.#inner.pendingStream.promise;
1740+
}
1741+
17221742
/**
17231743
* True if any data on this stream was received as 0-RTT (early data)
17241744
* before the TLS handshake completed. Early data is less secure and
@@ -2597,6 +2617,13 @@ class QuicStream {
25972617
} else {
25982618
inner.pendingClose.resolve();
25992619
}
2620+
if (inner.state.pending) {
2621+
if (error !== undefined) {
2622+
inner.pendingStream.reject(error);
2623+
} else {
2624+
inner.pendingStream.resolve(error);
2625+
}
2626+
}
26002627
debug('stream closed');
26012628
if (onStreamClosedChannel.hasSubscribers) {
26022629
onStreamClosedChannel.publish({
@@ -2642,6 +2669,12 @@ class QuicStream {
26422669
}
26432670
}
26442671

2672+
[kAvailable]() {
2673+
// The formerly pending stream is now available
2674+
const inner = this.#inner;
2675+
inner.pendingStream.resolve();
2676+
}
2677+
26452678
[kBlocked]() {
26462679
const inner = this.#inner;
26472680
// The blocked event should only be called if the stream was created with

lib/internal/quic/symbols.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ const {
2828
// public API.
2929

3030
const kAttachFileHandle = Symbol('kAttachFileHandle');
31+
const kAvailable = Symbol('kAvailable');
3132
const kBlocked = Symbol('kBlocked');
3233
const kConnect = Symbol('kConnect');
3334
const kDrain = Symbol('kDrain');
@@ -63,6 +64,7 @@ const kVersionNegotiation = Symbol('kVersionNegotiation');
6364

6465
module.exports = {
6566
kAttachFileHandle,
67+
kAvailable,
6668
kBlocked,
6769
kConnect,
6870
kDatagram,

src/quic/bindingdata.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ class SessionManager;
5555
V(session_path_validation, SessionPathValidation) \
5656
V(session_ticket, SessionTicket) \
5757
V(session_version_negotiation, SessionVersionNegotiation) \
58+
V(stream_available, StreamAvailable) \
5859
V(stream_blocked, StreamBlocked) \
5960
V(stream_close, StreamClose) \
6061
V(stream_created, StreamCreated) \

src/quic/streams.cc

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1195,10 +1195,13 @@ void Stream::NotifyStreamOpened(stream_id id) {
11951195
// sending can eventually call into JavaScript and destroy the stream.
11961196
BaseObjectPtr<Stream> self(this);
11971197
auto& application = session().application();
1198-
error_code internal_error = application.GetInternalErrorCode();
11991198
if (!application.StreamOpened(*this) && !is_destroyed()) {
1199+
error_code internal_error = application.GetInternalErrorCode();
12001200
Destroy(QuicError::ForApplication(internal_error));
12011201
}
1202+
1203+
// We inform, the js side that the pending stream is now available
1204+
if (!is_destroyed()) EmitStreamAvailable();
12021205
}
12031206

12041207
void Stream::NotifyReadableEnded(error_code code) {
@@ -1779,6 +1782,14 @@ void Stream::SendStopSending(error_code code) {
17791782

17801783
// ============================================================================
17811784

1785+
void Stream::EmitStreamAvailable() {
1786+
if (!env()->can_call_into_js()) {
1787+
return;
1788+
}
1789+
CallbackScope<Stream> cb_scope(this);
1790+
MakeCallback(BindingData::Get(env()).stream_available_callback(), 0, nullptr);
1791+
}
1792+
17821793
void Stream::EmitBlocked() {
17831794
// state()->wants_block will be set from the javascript side if the
17841795
// stream object has a handler for the blocked event.

src/quic/streams.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,10 @@ class Stream final : public AsyncWrap,
433433

434434
// JavaScript callouts
435435

436+
// Notifies the JavaScript side that a previously pending stream
437+
// is now available.
438+
void EmitStreamAvailable();
439+
436440
// Notifies the JavaScript side that the stream has been destroyed.
437441
void EmitClose(const QuicError& error);
438442

test/parallel/test-quic-internal-setcallbacks.mjs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ const callbacks = {
2626
onSessionOrigin() {},
2727
onSessionGoaway() {},
2828
onSessionVersionNegotiation() {},
29+
onStreamAvailable() {},
2930
onStreamCreated() {},
3031
onStreamBlocked() {},
3132
onStreamClose() {},

test/parallel/test-quic-stream-limits-pending.mjs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,28 +38,55 @@ const serverEndpoint = await listen(mustCall((serverSession) => {
3838
const clientSession = await connect(serverEndpoint.address);
3939
await clientSession.opened;
4040

41+
let opened = 0;
42+
4143
// First stream opens immediately (within the limit).
4244
const s1 = await clientSession.createBidirectionalStream({
4345
body: encoder.encode('stream 1'),
4446
});
4547

48+
// eslint-disable-next-line node-core/must-call-assert
49+
s1.opened.then(() => {
50+
opened++;
51+
});
52+
4653
// Second stream is created but queued as pending because the
4754
// server only allows 1 concurrent bidi stream.
4855
const s2 = await clientSession.createBidirectionalStream({
4956
body: encoder.encode('stream 2'),
5057
});
5158

59+
// eslint-disable-next-line node-core/must-call-assert
60+
s2.opened.then(() => {
61+
opened++;
62+
});
63+
64+
// Third stream is created but queued as pending because the
65+
// server only allows 1 concurrent bidi stream.
66+
const s3 = await clientSession.createBidirectionalStream({
67+
body: encoder.encode('stream 3'),
68+
});
69+
70+
5271
// s2 should be pending until s1 closes and the server grants
5372
// more stream credits.
5473
assert.strictEqual(s2.pending, true);
74+
assert.strictEqual(opened, 1);
5575

5676
// Drain and close the first stream.
5777
for await (const _ of s1) { /* drain */ } // eslint-disable-line no-unused-vars
5878
await s1.closed;
5979

80+
const err = new Error('Test error');
81+
s3.destroy(err);
82+
83+
await Promise.all([assert.rejects(s3.opened, err), assert.rejects(s3.closed, err)]);
84+
85+
6086
// After s1 closes, the server sends MAX_STREAMS which opens s2.
6187
// Wait for the server to receive both streams.
6288
await allDone.promise;
89+
assert.strictEqual(opened, 2);
6390

6491
// s2 should no longer be pending.
6592
for await (const _ of s2) { /* drain */ } // eslint-disable-line no-unused-vars

test/parallel/test-quic-stream-limits-uni.mjs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,20 +34,35 @@ const serverEndpoint = await listen(mustCall((serverSession) => {
3434
const clientSession = await connect(serverEndpoint.address);
3535
await clientSession.opened;
3636

37+
let opened = 0;
38+
3739
// First uni stream opens immediately.
3840
const s1 = await clientSession.createUnidirectionalStream({
3941
body: encoder.encode('uni 1'),
4042
});
4143

44+
// eslint-disable-next-line node-core/must-call-assert
45+
s1.opened.then(() => {
46+
opened++;
47+
});
48+
4249
// Second uni stream is pending (limit = 1).
4350
const s2 = await clientSession.createUnidirectionalStream({
4451
body: encoder.encode('uni 2'),
4552
});
53+
54+
// eslint-disable-next-line node-core/must-call-assert
55+
s2.opened.then(() => {
56+
opened++;
57+
});
58+
assert.strictEqual(opened, 1);
59+
4660
assert.strictEqual(s2.pending, true);
4761

4862
// Wait for both to complete.
4963
await s1.closed;
5064
await allDone.promise;
65+
assert.strictEqual(opened, 2);
5166
await s2.closed;
5267

5368
await clientSession.close();

typings/internalBinding/quic.d.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ interface QuicCallbacks {
1919
versions: number[],
2020
supports: number[]) => void;
2121
onStreamCreated: (stream: Stream) => void;
22+
onStreamAvailable: () => void;
2223
onStreamBlocked: () => void;
2324
onStreamClose: (error: [number,bigint,string]) => void;
2425
onStreamReset: (error: [number,bigint,string]) => void;

0 commit comments

Comments
 (0)