diff --git a/.changeset/cross-copy-instanceof.md b/.changeset/cross-copy-instanceof.md new file mode 100644 index 00000000..3b39249b --- /dev/null +++ b/.changeset/cross-copy-instanceof.md @@ -0,0 +1,5 @@ +--- +'@livekit/rtc-node': patch +--- + +Make `instanceof` work across a dual-loaded copy of the package for the plain data classes `AudioFrame`, `VideoFrame`, `RpcError` and `ConnectError`. When a process reaches this package through both `import` (`dist/index.js`) and `require` (`dist/index.cjs`) — which happens whenever a dependency uses `createRequire` — those are two separate module graphs with two separate class objects, so `frame instanceof AudioFrame` used to be `false` for a frame built by the other copy. These classes now brand their prototype with a `Symbol.for()` key from the global symbol registry and match on the brand. Classes that own an FFI handle (`Room`, `AudioStream`, `AudioSource`, participants, tracks, ...) are deliberately left on prototype identity. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4271900f..98928f56 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -64,6 +64,9 @@ jobs: run: pnpm install - name: Test build run: pnpm --filter="*livekit*" run build + - name: Test rtc-node dual package instanceof + # Needs the built dist/, since it loads both published entry points at once. + run: pnpm --filter="@livekit/rtc-node" run test:dual-package - name: Test livekit-server-sdk (Node) run: pnpm --filter="livekit-server-sdk" test - name: Test livekit-server-sdk (Browser) diff --git a/packages/livekit-rtc/package.json b/packages/livekit-rtc/package.json index b4ce7c47..d597b5d1 100644 --- a/packages/livekit-rtc/package.json +++ b/packages/livekit-rtc/package.json @@ -54,6 +54,7 @@ "build": "pnpm prebuild && tsup --onSuccess \"tsc --declaration --emitDeclarationOnly\"", "lint": "eslint -f unix \"src/**/*.ts\" --ignore-pattern \"src/proto/*\"", "test": "vitest run src", + "test:dual-package": "node scripts/verify-dual-package-instanceof.mjs", "test:e2e": "node scripts/run-e2e.mjs" } } diff --git a/packages/livekit-rtc/scripts/verify-dual-package-instanceof.mjs b/packages/livekit-rtc/scripts/verify-dual-package-instanceof.mjs new file mode 100644 index 00000000..4a894fc7 --- /dev/null +++ b/packages/livekit-rtc/scripts/verify-dual-package-instanceof.mjs @@ -0,0 +1,124 @@ +// SPDX-FileCopyrightText: 2026 LiveKit, Inc. +// +// SPDX-License-Identifier: Apache-2.0 +// Loads both published entry points of this package in a single process -- `dist/index.js` +// through `import` and `dist/index.cjs` through `createRequire` -- and checks that `instanceof` +// works across the two copies. This is the dual package hazard that unbranded classes hit; run +// `pnpm build` first. +// +// node scripts/verify-dual-package-instanceof.mjs +import { createRequire } from 'node:module'; +import { fileURLToPath } from 'node:url'; +import * as esm from '../dist/index.js'; + +const require = createRequire(import.meta.url); +const cjs = require('../dist/index.cjs'); + +let failures = 0; + +const check = (label, actual, expected) => { + const ok = actual === expected; + if (!ok) failures++; + console.log(`${ok ? 'ok ' : 'FAIL'} ${label}: ${actual} (expected ${expected})`); +}; + +console.log(`esm entry: ${fileURLToPath(new URL('../dist/index.js', import.meta.url))}`); +console.log(`cjs entry: ${require.resolve('../dist/index.cjs')}`); +console.log(''); + +// Sanity check: the two entry points really are two distinct copies of the same classes. +check('esm.AudioFrame !== cjs.AudioFrame', esm.AudioFrame !== cjs.AudioFrame, true); +check('esm.VideoFrame !== cjs.VideoFrame', esm.VideoFrame !== cjs.VideoFrame, true); +check('esm.RpcError !== cjs.RpcError', esm.RpcError !== cjs.RpcError, true); +console.log(''); + +const esmAudioFrame = esm.AudioFrame.create(48000, 1, 480); +const cjsAudioFrame = cjs.AudioFrame.create(48000, 1, 480); +check( + 'cjs-built AudioFrame instanceof esm.AudioFrame', + cjsAudioFrame instanceof esm.AudioFrame, + true, +); +check( + 'esm-built AudioFrame instanceof cjs.AudioFrame', + esmAudioFrame instanceof cjs.AudioFrame, + true, +); + +const esmVideoFrame = new esm.VideoFrame( + new Uint8Array(16 * 16 * 4), + 16, + 16, + esm.VideoBufferType.RGBA, +); +const cjsVideoFrame = new cjs.VideoFrame( + new Uint8Array(16 * 16 * 4), + 16, + 16, + cjs.VideoBufferType.RGBA, +); +check( + 'cjs-built VideoFrame instanceof esm.VideoFrame', + cjsVideoFrame instanceof esm.VideoFrame, + true, +); +check( + 'esm-built VideoFrame instanceof cjs.VideoFrame', + esmVideoFrame instanceof cjs.VideoFrame, + true, +); + +const esmRpcError = new esm.RpcError(1500, 'boom'); +const cjsRpcError = new cjs.RpcError(1500, 'boom'); +check('cjs-built RpcError instanceof esm.RpcError', cjsRpcError instanceof esm.RpcError, true); +check('esm-built RpcError instanceof cjs.RpcError', esmRpcError instanceof cjs.RpcError, true); + +const esmConnectError = new esm.ConnectError('boom'); +const cjsConnectError = new cjs.ConnectError('boom'); +check( + 'cjs-built ConnectError instanceof esm.ConnectError', + cjsConnectError instanceof esm.ConnectError, + true, +); +check( + 'esm-built ConnectError instanceof cjs.ConnectError', + esmConnectError instanceof cjs.ConnectError, + true, +); +console.log(''); + +// Branding must not make unrelated values match. +check('{} instanceof esm.AudioFrame', {} instanceof esm.AudioFrame, false); +check('null instanceof esm.AudioFrame', null instanceof esm.AudioFrame, false); +check('undefined instanceof esm.AudioFrame', undefined instanceof esm.AudioFrame, false); +check( + 'unbranded lookalike instanceof esm.AudioFrame', + { data: new Int16Array(480), sampleRate: 48000, channels: 1, samplesPerChannel: 480 } instanceof + esm.AudioFrame, + false, +); +check( + 'esm-built VideoFrame instanceof esm.AudioFrame', + esmVideoFrame instanceof esm.AudioFrame, + false, +); +check( + 'esm-built AudioFrame instanceof esm.VideoFrame', + esmAudioFrame instanceof esm.VideoFrame, + false, +); +console.log(''); + +// Handle-backed classes are deliberately left alone: they are live resources, not values. +check('esm.Room !== cjs.Room', esm.Room !== cjs.Room, true); +check( + 'cjs Room.prototype instanceof-safe brand absent', + Object.getOwnPropertySymbols(cjs.Room.prototype).some((s) => + s.description?.startsWith('lk.rtc-node.'), + ), + false, +); +console.log(''); + +console.log(failures === 0 ? 'PASS: all checks passed' : `FAIL: ${failures} check(s) failed`); +process.exit(failures === 0 ? 0 : 1); diff --git a/packages/livekit-rtc/src/audio_frame.ts b/packages/livekit-rtc/src/audio_frame.ts index 7da1e314..e3e4c4c6 100644 --- a/packages/livekit-rtc/src/audio_frame.ts +++ b/packages/livekit-rtc/src/audio_frame.ts @@ -3,6 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 import type { OwnedAudioFrameBuffer } from '@livekit/rtc-ffi-bindings'; import { AudioFrameBufferInfo } from '@livekit/rtc-ffi-bindings'; +import { brandDataClass } from './brand.js'; import { FfiClient, FfiHandle } from './ffi_client.js'; export class AudioFrame { @@ -72,6 +73,10 @@ export class AudioFrame { } } +// An AudioFrame is nothing but its fields, so it stays usable across a dual-loaded copy of this +// package. See ./brand.ts. +brandDataClass(AudioFrame, Symbol.for('lk.rtc-node.AudioFrame')); + /** * Combines one or more `rtc.AudioFrame` objects into a single `rtc.AudioFrame`. * diff --git a/packages/livekit-rtc/src/brand.test.ts b/packages/livekit-rtc/src/brand.test.ts new file mode 100644 index 00000000..ef135186 --- /dev/null +++ b/packages/livekit-rtc/src/brand.test.ts @@ -0,0 +1,160 @@ +// SPDX-FileCopyrightText: 2026 LiveKit, Inc. +// +// SPDX-License-Identifier: Apache-2.0 +import { VideoBufferType } from '@livekit/rtc-ffi-bindings'; +import { describe, expect, it } from 'vitest'; +import { AudioFrame } from './audio_frame.js'; +import { AudioResampler } from './audio_resampler.js'; +import { AudioSource } from './audio_source.js'; +import { AudioStream } from './audio_stream.js'; +import { brandDataClass } from './brand.js'; +import { LocalParticipant, RemoteParticipant } from './participant.js'; +import { ConnectError, Room } from './room.js'; +import { RpcError } from './rpc.js'; +import { Track } from './track.js'; +import { VideoFrame } from './video_frame.js'; + +type AnyClass = new (...args: never[]) => object; + +const AUDIO_FRAME_BRAND = 'lk.rtc-node.AudioFrame'; +const VIDEO_FRAME_BRAND = 'lk.rtc-node.VideoFrame'; + +/** + * Stands in for a class loaded from a second copy of this package: a distinct class object with a + * distinct prototype, carrying the same brand from the global symbol registry. + */ +const secondCopyOf = (brand: string): AnyClass => { + class Foreign {} + brandDataClass(Foreign, Symbol.for(brand)); + return Foreign; +}; + +const cases: Array<{ name: string; brand: string; Class: AnyClass; create: () => object }> = [ + { + name: 'AudioFrame', + brand: AUDIO_FRAME_BRAND, + Class: AudioFrame, + create: () => AudioFrame.create(48000, 1, 480), + }, + { + name: 'VideoFrame', + brand: VIDEO_FRAME_BRAND, + Class: VideoFrame, + create: () => new VideoFrame(new Uint8Array(4), 1, 1, VideoBufferType.RGBA), + }, + { + name: 'RpcError', + brand: 'lk.rtc-node.RpcError', + Class: RpcError, + create: () => new RpcError(1500, 'boom'), + }, + { + name: 'ConnectError', + brand: 'lk.rtc-node.ConnectError', + Class: ConnectError, + create: () => new ConnectError('boom'), + }, +]; + +describe.each(cases)('$name cross-copy instanceof', ({ brand, Class, create }) => { + it('matches a genuine local instance', () => { + expect(create()).toBeInstanceOf(Class); + }); + + it('matches an instance built by a second copy of the package', () => { + const Foreign = secondCopyOf(brand); + + expect(Foreign).not.toBe(Class); + expect(new Foreign()).toBeInstanceOf(Class); + }); + + it('does not match values that carry no brand', () => { + expect({}).not.toBeInstanceOf(Class); + expect(null).not.toBeInstanceOf(Class); + expect(undefined).not.toBeInstanceOf(Class); + expect('a string').not.toBeInstanceOf(Class); + expect(42).not.toBeInstanceOf(Class); + expect(Object.create(null)).not.toBeInstanceOf(Class); + expect(create).not.toBeInstanceOf(Class); + }); + + it('does not match a same-shaped but unbranded lookalike', () => { + expect({ ...create() }).not.toBeInstanceOf(Class); + }); + + it('does not match an instance branded as a different class', () => { + const Other = secondCopyOf(brand === AUDIO_FRAME_BRAND ? VIDEO_FRAME_BRAND : AUDIO_FRAME_BRAND); + + expect(new Other()).not.toBeInstanceOf(Class); + }); + + it('leaves subclasses on prototype identity', () => { + class Subclass extends Class {} + + // A subclass instance inherits the brand, so it is still an instance of the branded parent. + expect(new Subclass()).toBeInstanceOf(Class); + // The reverse must not hold, for local or foreign instances of the parent. + expect(create()).not.toBeInstanceOf(Subclass); + expect(new (secondCopyOf(brand))()).not.toBeInstanceOf(Subclass); + }); +}); + +describe('brandDataClass', () => { + it('resolves the same symbol from every copy of the module', () => { + expect(Symbol.for(AUDIO_FRAME_BRAND)).toBe(Symbol.for(AUDIO_FRAME_BRAND)); + }); + + it('brands the prototype, not each instance', () => { + const frame = AudioFrame.create(48000, 1, 480); + + expect(Object.getOwnPropertySymbols(AudioFrame.prototype)).toContain( + Symbol.for(AUDIO_FRAME_BRAND), + ); + expect(Object.getOwnPropertySymbols(frame)).toEqual([]); + }); + + it('keeps the brand out of enumerable properties', () => { + const frame = AudioFrame.create(48000, 1, 480); + + expect(Object.keys(frame)).toEqual([ + 'data', + 'sampleRate', + 'channels', + 'samplesPerChannel', + '_userdata', + ]); + expect(Object.getOwnPropertySymbols({ ...frame })).toEqual([]); + }); + + it('does not disturb the fields of a branded class', () => { + const frame = new AudioFrame(new Int16Array([1, 2]), 48000, 1, 2, { source: 'test' }); + + expect(frame.data).toEqual(new Int16Array([1, 2])); + expect(frame.sampleRate).toBe(48000); + expect(frame.channels).toBe(1); + expect(frame.samplesPerChannel).toBe(2); + expect(frame.userdata).toEqual({ source: 'test' }); + }); +}); + +describe('classes that wrap a live FFI resource', () => { + // Two copies of these are genuinely not interchangeable -- they own handles, event wiring and + // internal state that the other copy's methods do not know about -- so they must keep failing + // `instanceof` across a copy boundary rather than failing more quietly somewhere downstream. + it.each([ + ['Room', Room], + ['AudioStream', AudioStream], + ['AudioSource', AudioSource], + ['AudioResampler', AudioResampler], + ['LocalParticipant', LocalParticipant], + ['RemoteParticipant', RemoteParticipant], + ['Track', Track], + ] as const)('leaves %s unbranded', (_name, ctor) => { + expect(Object.getOwnPropertyDescriptor(ctor, Symbol.hasInstance)).toBeUndefined(); + expect( + Object.getOwnPropertySymbols(ctor.prototype).filter((symbol) => + symbol.description?.startsWith('lk.rtc-node.'), + ), + ).toEqual([]); + }); +}); diff --git a/packages/livekit-rtc/src/brand.ts b/packages/livekit-rtc/src/brand.ts new file mode 100644 index 00000000..8d2230a4 --- /dev/null +++ b/packages/livekit-rtc/src/brand.ts @@ -0,0 +1,64 @@ +// SPDX-FileCopyrightText: 2026 LiveKit, Inc. +// +// SPDX-License-Identifier: Apache-2.0 + +/** + * A single process can end up with two copies of this package loaded at once: `dist/index.js` + * (resolved through the `import` condition) and `dist/index.cjs` (resolved through `require`) are + * separate module graphs, so a dependency that reaches the package with `require` gets a different + * `AudioFrame` class than one that reaches it with `import`, even though only one copy is + * installed on disk. Node calls this the dual package hazard. + * + * `instanceof` compares prototype identity, so a frame produced by one copy fails `instanceof` + * in the other despite the two classes being compiled from the same source. + * + * For classes whose entire meaning is their fields we can key `instanceof` on a brand instead of + * on prototype identity. `Symbol.for` looks up the global symbol registry, which is shared by + * every copy of a module in the process, so the brand is the same symbol everywhere. This is the + * same technique as `Symbol.for('react.element')`, `Symbol.for('nodejs.util.inspect.custom')` and + * the existing `Symbol.for('lk.frame-processor')` in this package. + */ + +/** Anything with a prototype we can hang a brand on. */ +type DataClass = abstract new (...args: never[]) => object; + +/** + * Makes `instanceof ctor` succeed for instances built by another copy of this package. + * + * Only call this for value types -- objects whose whole contract is their public fields, such as + * `AudioFrame`. Do **not** call it for classes that own an FFI handle or otherwise wrap a + * live resource (`Room`, `AudioStream`, `AudioSource`, `AudioResampler`, participants, tracks, + * ...). The brand only asserts "this was built by some copy of this class", which for a value type + * is the whole story, but for a live resource says nothing about the things that actually have to + * match: handle ownership and disposal, event wiring, and the internal state the other copy's + * methods assume. Letting those cross the boundary would replace a loud `instanceof` failure with + * a subtler one further downstream. + * + * @internal + */ +export const brandDataClass = (ctor: DataClass, brand: symbol): void => { + Object.defineProperty(ctor.prototype, brand, { + value: true, + enumerable: false, + writable: false, + configurable: false, + }); + + Object.defineProperty(ctor, Symbol.hasInstance, { + value: function (this: unknown, value: unknown): boolean { + // Statics are inherited, so a subclass would otherwise match every branded instance of its + // parent. Anything that isn't the branded class itself gets the default behaviour. + if (this !== ctor) { + return Function.prototype[Symbol.hasInstance].call(this, value); + } + return ( + value !== null && + typeof value === 'object' && + (value as Record)[brand] === true + ); + }, + enumerable: false, + writable: false, + configurable: false, + }); +}; diff --git a/packages/livekit-rtc/src/room.ts b/packages/livekit-rtc/src/room.ts index 6ee23de0..bd85f683 100644 --- a/packages/livekit-rtc/src/room.ts +++ b/packages/livekit-rtc/src/room.ts @@ -37,6 +37,7 @@ import { import { TrackKind } from '@livekit/rtc-ffi-bindings'; import type { TypedEventEmitter as TypedEmitter } from '@livekit/typed-emitter'; import EventEmitter from 'events'; +import { brandDataClass } from './brand.js'; import { ByteStreamReader, TextStreamReader } from './data_streams/stream_reader.js'; import type { ByteStreamHandler, @@ -1033,6 +1034,11 @@ export class ConnectError extends Error { } } +// Callers catch this around `Room.connect`, so it crosses into user code the same way RpcError +// does. Note this brands the error, not Room itself: Room owns an FFI handle and is deliberately +// left alone. See ./brand.ts. +brandDataClass(ConnectError, Symbol.for('lk.rtc-node.ConnectError')); + export type RoomCallbacks = { participantConnected: (participant: RemoteParticipant) => void; participantDisconnected: (participant: RemoteParticipant) => void; diff --git a/packages/livekit-rtc/src/rpc.ts b/packages/livekit-rtc/src/rpc.ts index 5682aa34..f19e1d99 100644 --- a/packages/livekit-rtc/src/rpc.ts +++ b/packages/livekit-rtc/src/rpc.ts @@ -2,6 +2,7 @@ // // SPDX-License-Identifier: Apache-2.0 import type { RpcError as RpcError_Proto } from '@livekit/rtc-ffi-bindings'; +import { brandDataClass } from './brand.js'; /** Parameters for initiating an RPC call */ export interface PerformRpcParams { @@ -119,3 +120,8 @@ export class RpcError extends Error { return new RpcError(RpcError.ErrorCode[key], RpcError.ErrorMessage[key], data); } } + +// RpcError is thrown by application RPC handlers and matched with `instanceof` in +// LocalParticipant.handleRpcMethodInvocation. The handler lives in user code, which is exactly +// where a second copy of this package shows up, so the match has to survive it. See ./brand.ts. +brandDataClass(RpcError, Symbol.for('lk.rtc-node.RpcError')); diff --git a/packages/livekit-rtc/src/video_frame.ts b/packages/livekit-rtc/src/video_frame.ts index 012a650b..e8bcd2c8 100644 --- a/packages/livekit-rtc/src/video_frame.ts +++ b/packages/livekit-rtc/src/video_frame.ts @@ -7,6 +7,7 @@ import { VideoBufferInfo_ComponentInfo, VideoBufferType, } from '@livekit/rtc-ffi-bindings'; +import { brandDataClass } from './brand.js'; import { FfiClient, FfiHandle, FfiRequest } from './ffi_client.js'; export class VideoFrame { @@ -103,6 +104,10 @@ export class VideoFrame { } } +// Like AudioFrame, a VideoFrame is a plain buffer plus its dimensions -- it holds no FFI handle of +// its own, so it survives crossing a copy boundary. See ./brand.ts. +brandDataClass(VideoFrame, Symbol.for('lk.rtc-node.VideoFrame')); + const getPlaneLength = (type: VideoBufferType, width: number, height: number): number => { const chromaWidth = Math.trunc((width + 1) / 2); const chromaHeight = Math.trunc((height + 1) / 2);