Skip to content

Commit 11f4c37

Browse files
kixelatedcodex
andauthored
fix(moq-net): resolve reordered track aliases (moq-dev#2262)
Co-authored-by: OpenAI Codex <codex@openai.com>
1 parent 76e768c commit 11f4c37

6 files changed

Lines changed: 252 additions & 63 deletions

File tree

js/net/src/ietf/adapter.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ export interface Session {
1313
openNativeBi?(): Promise<Stream>;
1414
acceptBi(): Promise<Stream | undefined>;
1515
nextRequestId(): Promise<bigint | undefined>;
16-
close?(): void;
16+
close(): void;
1717
readonly version: IetfVersion;
1818
}
1919

@@ -44,6 +44,11 @@ export class NativeSession implements Session {
4444
this.#requestId += 2n;
4545
return id;
4646
}
47+
48+
/** Closes the underlying WebTransport session. */
49+
close() {
50+
this.#quic.close();
51+
}
4752
}
4853

4954
// Route classification for control stream messages.

js/net/src/ietf/aliases.test.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import { expect, test } from "bun:test";
2+
import { TrackAliases } from "./aliases.ts";
3+
4+
test("waits for the control message that establishes an alias", async () => {
5+
const aliases = new TrackAliases<object>();
6+
const track = {};
7+
const pending = aliases.get(7n);
8+
9+
aliases.set(7n, track);
10+
11+
expect(await pending).toBe(track);
12+
});
13+
14+
test("resolves every subgroup waiting for the same alias", async () => {
15+
const aliases = new TrackAliases<object>();
16+
const track = {};
17+
const pending = [aliases.get(7n), aliases.get(7n)];
18+
19+
aliases.set(7n, track);
20+
21+
expect(await Promise.all(pending)).toEqual([track, track]);
22+
});
23+
24+
test("rejects an alias used by two active tracks", () => {
25+
const aliases = new TrackAliases<object>();
26+
aliases.set(7n, {});
27+
28+
expect(() => aliases.set(7n, {})).toThrow("duplicate track alias");
29+
});
30+
31+
test("does not let stale cleanup remove a reused alias", async () => {
32+
const aliases = new TrackAliases<object>();
33+
const active = {};
34+
aliases.set(7n, active);
35+
36+
aliases.delete(7n, {});
37+
38+
expect(await aliases.get(7n)).toBe(active);
39+
});

js/net/src/ietf/aliases.ts

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
const TRACK_ALIAS_TIMEOUT_MS = 1000;
2+
3+
type Resolver<T> = PromiseWithResolvers<T>["resolve"];
4+
5+
/** Resolves publisher-chosen track aliases after control/data stream reordering. @internal */
6+
export class TrackAliases<T> {
7+
#active = new Map<bigint, T>();
8+
#pending = new Map<bigint, Set<Resolver<T>>>();
9+
10+
/** Waits briefly for an alias to be established by SUBSCRIBE_OK or PUBLISH. */
11+
async get(alias: bigint): Promise<T> {
12+
if (this.#active.has(alias)) return this.#active.get(alias) as T;
13+
14+
const { promise, resolve } = Promise.withResolvers<T>();
15+
let resolvers = this.#pending.get(alias);
16+
if (!resolvers) {
17+
resolvers = new Set();
18+
this.#pending.set(alias, resolvers);
19+
}
20+
resolvers.add(resolve);
21+
22+
let timer: ReturnType<typeof setTimeout> | undefined;
23+
const timeout = new Promise<never>((_, reject) => {
24+
timer = setTimeout(() => reject(new Error(`unknown track alias: ${alias}`)), TRACK_ALIAS_TIMEOUT_MS);
25+
});
26+
27+
try {
28+
return await Promise.race([promise, timeout]);
29+
} finally {
30+
clearTimeout(timer);
31+
resolvers.delete(resolve);
32+
if (this.#pending.get(alias) === resolvers && resolvers.size === 0) this.#pending.delete(alias);
33+
}
34+
}
35+
36+
/** Establishes an alias and releases any data streams waiting for it. */
37+
set(alias: bigint, value: T) {
38+
const active = this.#active.get(alias);
39+
if (this.#active.has(alias)) {
40+
if (active !== value) throw new Error(`duplicate track alias: ${alias}`);
41+
return;
42+
}
43+
44+
this.#active.set(alias, value);
45+
const resolvers = this.#pending.get(alias);
46+
this.#pending.delete(alias);
47+
for (const resolve of resolvers ?? []) resolve(value);
48+
}
49+
50+
/** Removes an alias only if it still belongs to the supplied value. */
51+
delete(alias: bigint, value: T) {
52+
if (this.#active.get(alias) === value) this.#active.delete(alias);
53+
}
54+
}

js/net/src/ietf/connection.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ export class Connection implements Established {
9898

9999
this.#closed = true;
100100

101-
this.#session.close?.();
101+
this.#session.close();
102102

103103
try {
104104
this.#quic.close();

js/net/src/ietf/subscriber.ts

Lines changed: 14 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import type { Reader, Stream } from "../stream.ts";
66
import type { Track } from "../track.ts";
77
import { error } from "../util/error.ts";
88
import type { Session } from "./adapter.ts";
9+
import { TrackAliases } from "./aliases.ts";
910
import { Frame, type Group as GroupMessage } from "./object.ts";
1011
import { type Publish, PublishError } from "./publish.ts";
1112
import { type PublishNamespace, PublishNamespaceError, PublishNamespaceOk } from "./publish_namespace.ts";
@@ -31,8 +32,8 @@ import { Version } from "./version.ts";
3132
export class Subscriber {
3233
#session: Session;
3334

34-
// Our subscribed tracks — keyed by trackAlias for group routing
35-
#subscribes = new Map<bigint, Track>();
35+
// Publisher-chosen aliases used by incoming group streams.
36+
#aliases = new TrackAliases<Track>();
3637

3738
// Any currently active announcements.
3839
#announced = new Set<Path.Valid>();
@@ -219,20 +220,19 @@ export class Subscriber {
219220
await msg.encode(stream.writer, version);
220221
console.debug(`subscribe written: id=${requestId} broadcast=${broadcast} track=${request.track.name}`);
221222

222-
// Pre-register with requestId so early group uni streams aren't dropped.
223-
// The publisher typically uses requestId as the trackAlias.
224-
this.#subscribes.set(requestId, request.track);
225-
226223
// Read response (SubscribeOk or error)
227224
const respTypeId = await stream.reader.u53();
228225
if (respTypeId === SubscribeOk.id) {
229226
const ok = await SubscribeOk.decode(stream.reader, version);
230-
// Update registration to use the actual trackAlias from SubscribeOk
231-
if (ok.trackAlias !== requestId) {
232-
this.#subscribes.delete(requestId);
233-
this.#subscribes.set(ok.trackAlias, request.track);
227+
try {
228+
this.#aliases.set(ok.trackAlias, request.track);
229+
} catch (err) {
230+
this.#session.close();
231+
throw err;
234232
}
235-
console.debug(`subscribe ok: id=${requestId} broadcast=${broadcast} track=${request.track.name}`);
233+
console.debug(
234+
`subscribe ok: id=${requestId} broadcast=${broadcast} track=${request.track.name} alias=${ok.trackAlias}`,
235+
);
236236

237237
try {
238238
// Wait for stream close (= PublishDone) or track close (= local unsubscribe)
@@ -259,12 +259,9 @@ export class Subscriber {
259259
`subscribe close: id=${requestId} broadcast=${broadcast} track=${request.track.name}`,
260260
);
261261
} finally {
262-
this.#subscribes.delete(ok.trackAlias);
262+
this.#aliases.delete(ok.trackAlias, request.track);
263263
}
264264
} else {
265-
// Clean up pre-registered entry on error
266-
this.#subscribes.delete(requestId);
267-
268265
// Error response
269266
let reasonPhrase = "unknown error";
270267
try {
@@ -282,7 +279,6 @@ export class Subscriber {
282279
throw new Error(`SUBSCRIBE error: ${reasonPhrase}`);
283280
}
284281
} catch (err) {
285-
this.#subscribes.delete(requestId);
286282
stream.abort(error(err));
287283
throw err;
288284
}
@@ -417,12 +413,8 @@ export class Subscriber {
417413
}
418414

419415
try {
420-
// Look up by trackAlias directly
421-
const track = this.#subscribes.get(group.trackAlias);
422-
if (!track) {
423-
// Fallback: try treating trackAlias as requestId (for compat)
424-
throw new Error(`unknown track: trackAlias=${group.trackAlias}`);
425-
}
416+
// The control message establishing this alias can arrive after the data stream.
417+
const track = await this.#aliases.get(group.trackAlias);
426418

427419
track.writeGroup(producer);
428420

0 commit comments

Comments
 (0)