From 750bea85731c366e4634d98a6dd53f970a398f76 Mon Sep 17 00:00:00 2001 From: Simon Corsin Date: Thu, 23 Jul 2026 13:10:20 -0500 Subject: [PATCH 1/3] Add MessageChannel and aggressive worker termination --- .../valdi/valdi_core/src/ValdiRuntime.d.ts | 14 +- .../src/valdi/worker/src/Worker.ts | 10 +- .../valdi/worker/test/MessageChannel.spec.ts | 168 ++++++ .../src/valdi/worker/test/WorkerTest.ts | 68 +++ .../test_workers/MessageChannelWorker.ts | 27 + .../valdi/hermes/HermesJavaScriptCompiler.cpp | 1 + .../valdi/hermes/HermesJavaScriptContext.cpp | 9 + .../valdi/hermes/HermesJavaScriptContext.hpp | 1 + valdi/src/valdi/hermes/HermesUtils.cpp | 24 +- .../src/valdi/jscore/JSCoreCustomClasses.cpp | 25 +- .../valdi/jscore/JavaScriptCoreContext.cpp | 8 + .../valdi/jscore/JavaScriptCoreContext.hpp | 3 + .../quickjs/QuickJSJavaScriptContext.cpp | 10 +- .../quickjs/QuickJSJavaScriptContext.hpp | 1 + valdi/src/valdi/quickjs/QuickJSUtils.cpp | 26 +- .../runtime/Interfaces/IJavaScriptContext.cpp | 29 +- .../runtime/Interfaces/IJavaScriptContext.hpp | 5 +- .../JavaScript/JavaScriptMessagePort.cpp | 511 ++++++++++++++++++ .../JavaScript/JavaScriptMessagePort.hpp | 122 +++++ .../runtime/JavaScript/JavaScriptRuntime.cpp | 175 ++++-- .../runtime/JavaScript/JavaScriptRuntime.hpp | 4 + .../runtime/JavaScript/JavaScriptUtils.cpp | 37 +- .../runtime/JavaScript/JavaScriptUtils.hpp | 13 + .../runtime/JavaScript/JavaScriptWorker.cpp | 154 ++++-- .../runtime/JavaScript/JavaScriptWorker.hpp | 32 +- valdi/test/integration/JSIntegrationTests.cpp | 112 ++++ valdi/test/integration/Runtime_tests.cpp | 48 ++ .../modules/test/src/InfiniteWorker.ts | 4 + .../resources/modules/test/src/WorkerTest.tsx | 39 +- .../cpp/Threading/GCDDispatchQueue.cpp | 4 + 30 files changed, 1520 insertions(+), 164 deletions(-) create mode 100644 src/valdi_modules/src/valdi/worker/test/MessageChannel.spec.ts create mode 100644 src/valdi_modules/src/valdi/worker/test_workers/MessageChannelWorker.ts create mode 100644 valdi/src/valdi/runtime/JavaScript/JavaScriptMessagePort.cpp create mode 100644 valdi/src/valdi/runtime/JavaScript/JavaScriptMessagePort.hpp create mode 100644 valdi/testdata/resources/modules/test/src/InfiniteWorker.ts diff --git a/src/valdi_modules/src/valdi/valdi_core/src/ValdiRuntime.d.ts b/src/valdi_modules/src/valdi/valdi_core/src/ValdiRuntime.d.ts index f9b26acf..3c2883f7 100644 --- a/src/valdi_modules/src/valdi/valdi_core/src/ValdiRuntime.d.ts +++ b/src/valdi_modules/src/valdi/valdi_core/src/ValdiRuntime.d.ts @@ -28,14 +28,22 @@ export interface ColorPalette { [key: string]: string; } -interface MessageEvent { +export interface NativeMessageEvent { readonly data: T; + readonly ports: readonly NativeMessagePort[]; } -export type OnMessageFunc = (msg: MessageEvent) => void; +export type OnMessageFunc = (msg: NativeMessageEvent) => void; + +export interface NativeMessagePort { + onmessage: OnMessageFunc | null; + postMessage(data: T, transfer?: readonly NativeMessagePort[]): void; + start(): void; + close(): void; +} export interface NativeWorker { - postMessage(data: T): void; + postMessage(data: T, transfer?: readonly NativeMessagePort[]): void; setOnMessage(f: OnMessageFunc): void; terminate(): void; } diff --git a/src/valdi_modules/src/valdi/worker/src/Worker.ts b/src/valdi_modules/src/valdi/worker/src/Worker.ts index 86506571..cd16a850 100644 --- a/src/valdi_modules/src/valdi/worker/src/Worker.ts +++ b/src/valdi_modules/src/valdi/worker/src/Worker.ts @@ -1,4 +1,4 @@ -import { ValdiRuntime, NativeWorker, OnMessageFunc } from 'valdi_core/src/ValdiRuntime'; +import type { NativeMessagePort, NativeWorker, OnMessageFunc, ValdiRuntime } from 'valdi_core/src/ValdiRuntime'; declare const runtime: ValdiRuntime; @@ -13,15 +13,15 @@ export class Worker { this.nativeWorker = runtime.createWorker(url); } - public set onmessage(func: OnMessageFunc) { + public set onmessage(func: (event: MessageEvent) => void) { if (this.nativeWorker) { - this.nativeWorker.setOnMessage(func); + this.nativeWorker.setOnMessage(func as OnMessageFunc); } } - public postMessage(data: T): void { + public postMessage(data: T, transfer?: readonly MessagePort[]): void { if (this.nativeWorker) { - this.nativeWorker.postMessage(data); + this.nativeWorker.postMessage(data, transfer as readonly NativeMessagePort[] | undefined); } } diff --git a/src/valdi_modules/src/valdi/worker/test/MessageChannel.spec.ts b/src/valdi_modules/src/valdi/worker/test/MessageChannel.spec.ts new file mode 100644 index 00000000..5372046e --- /dev/null +++ b/src/valdi_modules/src/valdi/worker/test/MessageChannel.spec.ts @@ -0,0 +1,168 @@ +import 'jasmine/src/jasmine'; + +interface NestedTransferredPortData { + readonly ports: readonly MessagePort[]; +} + +interface TransferredPortData { + readonly nested: NestedTransferredPortData; +} + +function receiveNext(port: MessagePort): Promise> { + return new Promise(resolve => { + port.onmessage = event => resolve(event as MessageEvent); + }); +} + +function nextTask(): Promise { + return new Promise(resolve => { + // eslint-disable-next-line @snap/valdi/assign-timer-id + setTimeout(resolve, 0); + }); +} + +describe('MessageChannel', () => { + it('is backed by native MessageChannel and MessagePort classes', () => { + const channel = new MessageChannel(); + + expect(channel instanceof MessageChannel).toBeTrue(); + expect(channel.port1).toBe(channel.port1); + expect(Object.prototype.hasOwnProperty.call(channel, 'port1')).toBeTrue(); + expect(Object.prototype.hasOwnProperty.call(channel, 'port2')).toBeTrue(); + expect(Object.getOwnPropertyDescriptor(channel, 'port1')?.writable).toBeTrue(); + expect(Object.getOwnPropertyDescriptor(channel, 'port2')?.writable).toBeTrue(); + expect(Object.prototype.hasOwnProperty.call(channel.port1, 'postMessage')).toBeFalse(); + expect(typeof Object.getPrototypeOf(channel.port1).postMessage).toBe('function'); + expect(channel.port1.onmessage).toBeNull(); + + const onmessage = () => {}; + channel.port1.onmessage = onmessage; + expect(channel.port1.onmessage).toBe(onmessage); + channel.port1.onmessage = null; + expect(channel.port1.onmessage).toBeNull(); + }); + + it('queues messages until onmessage starts the port and preserves FIFO order', async () => { + const channel = new MessageChannel(); + const messages: unknown[] = []; + + channel.port1.postMessage('first'); + channel.port1.postMessage('second', []); + channel.port1.postMessage('third'); + + const received = new Promise(resolve => { + channel.port2.onmessage = event => { + messages.push(event.data); + expect(event.ports).toEqual([]); + if (messages.length === 3) { + resolve(); + } + }; + }); + + await received; + expect(messages).toEqual(['first', 'second', 'third']); + }); + + it('delivers ArrayBuffer values', async () => { + const channel = new MessageChannel(); + const received = receiveNext(channel.port2); + const buffer = new Uint8Array([1, 2, 3]).buffer; + + channel.port1.postMessage(buffer); + + const event = await received; + expect(Array.from(new Uint8Array(event.data))).toEqual([1, 2, 3]); + }); + + it('discards messages delivered after start when there is no handler', async () => { + const channel = new MessageChannel(); + channel.port2.start(); + channel.port1.postMessage('discarded'); + await nextTask(); + + const received = receiveNext(channel.port2); + channel.port1.postMessage('delivered'); + expect((await received).data).toBe('delivered'); + }); + + it('can transfer a port over another port and queue messages during transfer', async () => { + const control = new MessageChannel(); + const payload = new MessageChannel(); + const transferredEvent = receiveNext(control.port2); + + control.port1.postMessage('transfer', [payload.port1]); + payload.port2.postMessage('queued immediately'); + + const event = await transferredEvent; + expect(event.data).toBe('transfer'); + expect(event.ports.length).toBe(1); + expect((await receiveNext(event.ports[0])).data).toBe('queued immediately'); + + // Detached handles are inert, including repeated close calls. + payload.port1.postMessage('ignored'); + payload.port1.start(); + payload.port1.close(); + payload.port1.close(); + expect(() => control.port1.postMessage('invalid', [payload.port1])).toThrowError( + 'MessagePort in transfer list is already detached', + ); + }); + + it('preserves transferred ports embedded in message data', async () => { + const control = new MessageChannel(); + const payload = new MessageChannel(); + const transferredEvent = receiveNext(control.port2); + + control.port1.postMessage({ nested: { ports: [payload.port1, payload.port1] } }, [payload.port1]); + + const event = await transferredEvent; + expect(event.ports.length).toBe(1); + expect(event.data.nested.ports[0]).toBe(event.ports[0]); + expect(event.data.nested.ports[1]).toBe(event.ports[0]); + + const received = receiveNext(event.data.nested.ports[0]); + payload.port2.postMessage('sent through data port'); + expect((await received).data).toBe('sent through data port'); + }); + + it('rejects duplicate transfers atomically', async () => { + const control = new MessageChannel(); + const payload = new MessageChannel(); + + expect(() => control.port1.postMessage('invalid', [payload.port1, payload.port1])).toThrowError( + 'Transfer list contains duplicate MessagePort', + ); + + const received = receiveNext(payload.port2); + payload.port1.postMessage('still attached'); + expect((await received).data).toBe('still attached'); + }); + + it('rejects transferring the sending port without detaching it', async () => { + const channel = new MessageChannel(); + + expect(() => channel.port1.postMessage('invalid', [channel.port1])).toThrowError( + 'Transfer list contains source MessagePort', + ); + + const received = receiveNext(channel.port2); + channel.port1.postMessage('still attached'); + expect((await received).data).toBe('still attached'); + }); + + it('closes ports idempotently and discards later messages', async () => { + const channel = new MessageChannel(); + let delivered = false; + channel.port2.onmessage = () => { + delivered = true; + }; + channel.port2.close(); + channel.port2.close(); + channel.port2.start(); + channel.port1.postMessage('ignored'); + + await nextTask(); + expect(delivered).toBeFalse(); + }); +}); diff --git a/src/valdi_modules/src/valdi/worker/test/WorkerTest.ts b/src/valdi_modules/src/valdi/worker/test/WorkerTest.ts index 9f2f93aa..22037c8b 100644 --- a/src/valdi_modules/src/valdi/worker/test/WorkerTest.ts +++ b/src/valdi_modules/src/valdi/worker/test/WorkerTest.ts @@ -1,11 +1,22 @@ import 'jasmine/src/jasmine'; import { Worker, inWorker } from 'worker/src/Worker'; +interface WorkerPortMessage { + readonly type: string; + readonly port: MessagePort; +} + function timeout(ms: number): Promise { // eslint-disable-next-line @snap/valdi/assign-timer-id return new Promise(resolve => setTimeout(resolve, ms, 'timeout')); } +function receiveNext(port: MessagePort): Promise> { + return new Promise(resolve => { + port.onmessage = event => resolve(event as MessageEvent); + }); +} + describe('worker', () => { it('run_with_worker', async () => { const worker = new Worker('worker/test_workers/TestWorker'); @@ -47,4 +58,61 @@ describe('worker', () => { }); expect(await pong).toEqual(echoValue); }, 100); + + it('communicates both ways over a channel transferred to a worker', async () => { + const worker = new Worker('worker/test_workers/MessageChannelWorker'); + const channel = new MessageChannel(); + const replyEvents: MessageEvent[] = []; + const replies = new Promise(resolve => { + channel.port1.onmessage = event => { + replyEvents.push(event); + if (replyEvents.length === 3) { + resolve(); + } + }; + }); + + try { + worker.postMessage({ type: 'initialize', port: channel.port2 }, [channel.port2]); + channel.port1.postMessage('first'); + channel.port1.postMessage('second'); + channel.port1.postMessage('third'); + + await replies; + expect(replyEvents.map(event => event.data)).toEqual([ + ['worker reply', 'first', 0, true], + ['worker reply', 'second', 0, true], + ['worker reply', 'third', 0, true], + ]); + expect(replyEvents.map(event => event.ports.length)).toEqual([0, 0, 0]); + } finally { + worker.terminate(); + } + }); + + it('communicates both ways over a channel transferred from a worker', async () => { + const worker = new Worker('worker/test_workers/MessageChannelWorker'); + const transferred = new Promise>(resolve => { + worker.onmessage = event => resolve(event); + }); + + try { + worker.postMessage('transfer from worker'); + + const event = await transferred; + const data = event.data as WorkerPortMessage; + expect(data.type).toBe('worker channel'); + expect(event.ports.length).toBe(1); + expect(data.port).toBe(event.ports[0]); + + const port = data.port; + expect((await receiveNext(port)).data).toBe('queued in worker'); + + const reply = receiveNext(port); + port.postMessage('sent to worker'); + expect((await reply).data).toEqual(['worker channel reply', 'sent to worker']); + } finally { + worker.terminate(); + } + }); }); diff --git a/src/valdi_modules/src/valdi/worker/test_workers/MessageChannelWorker.ts b/src/valdi_modules/src/valdi/worker/test_workers/MessageChannelWorker.ts new file mode 100644 index 00000000..e8f59d78 --- /dev/null +++ b/src/valdi_modules/src/valdi/worker/test_workers/MessageChannelWorker.ts @@ -0,0 +1,27 @@ +interface WorkerPortMessage { + readonly type: string; + readonly port: MessagePort; +} + +onmessage = workerEvent => { + const event = workerEvent as unknown as MessageEvent; + if (event.data === 'transfer from worker') { + const channel = new MessageChannel(); + const postWorkerMessage = postMessage as (data: unknown, transfer?: readonly MessagePort[]) => void; + channel.port1.onmessage = portEvent => { + channel.port1.postMessage(['worker channel reply', portEvent.data]); + }; + postWorkerMessage({ type: 'worker channel', port: channel.port2 }, [channel.port2]); + channel.port1.postMessage('queued in worker'); + return; + } + + const port = (event.data as WorkerPortMessage).port; + const dataPortMatchesTransferredPort = port === event.ports[0]; + + // The transferred port becomes the worker's only message receiver. + delete (globalThis as { onmessage?: unknown }).onmessage; + port.onmessage = portEvent => { + port.postMessage(['worker reply', portEvent.data, portEvent.ports.length, dataPortMatchesTransferredPort]); + }; +}; diff --git a/valdi/src/valdi/hermes/HermesJavaScriptCompiler.cpp b/valdi/src/valdi/hermes/HermesJavaScriptCompiler.cpp index a5306727..484a1d0f 100644 --- a/valdi/src/valdi/hermes/HermesJavaScriptCompiler.cpp +++ b/valdi/src/valdi/hermes/HermesJavaScriptCompiler.cpp @@ -96,6 +96,7 @@ static hermes::hbc::CompileFlags getCompileFlags(hermes::OutputFormatKind output compileFlags.enableBlockScoping = true; compileFlags.enableES6Classes = true; compileFlags.debug = debug; + compileFlags.emitAsyncBreakCheck = true; return compileFlags; } diff --git a/valdi/src/valdi/hermes/HermesJavaScriptContext.cpp b/valdi/src/valdi/hermes/HermesJavaScriptContext.cpp index 9bc09374..03d3ee12 100644 --- a/valdi/src/valdi/hermes/HermesJavaScriptContext.cpp +++ b/valdi/src/valdi/hermes/HermesJavaScriptContext.cpp @@ -1393,10 +1393,19 @@ void HermesJavaScriptContext::willEnterVM() { _enterVMCount++; } +void HermesJavaScriptContext::requestExecutionTermination() { + IJavaScriptContext::requestExecutionTermination(); + _runtime->triggerTimeoutAsyncBreak(); +} + void HermesJavaScriptContext::willExitVM(JSExceptionTracker& exceptionTracker) { SC_ASSERT(_enterVMCount > 0); --_enterVMCount; if (_enterVMCount == 0) { + if (executionTerminationRequested()) { + return; + } + auto result = _runtime->drainJobs(); checkException(result, exceptionTracker); } diff --git a/valdi/src/valdi/hermes/HermesJavaScriptContext.hpp b/valdi/src/valdi/hermes/HermesJavaScriptContext.hpp index 9362e68a..a3d83d6e 100644 --- a/valdi/src/valdi/hermes/HermesJavaScriptContext.hpp +++ b/valdi/src/valdi/hermes/HermesJavaScriptContext.hpp @@ -153,6 +153,7 @@ class HermesJavaScriptContext : public IJavaScriptContext { void willEnterVM() final; void willExitVM(JSExceptionTracker& exceptionTracker) final; + void requestExecutionTermination() final; void startDebugger(bool isWorker) final; std::optional getDebuggerInfo() const final; diff --git a/valdi/src/valdi/hermes/HermesUtils.cpp b/valdi/src/valdi/hermes/HermesUtils.cpp index dffbeb8a..0cbfdfb3 100644 --- a/valdi/src/valdi/hermes/HermesUtils.cpp +++ b/valdi/src/valdi/hermes/HermesUtils.cpp @@ -86,6 +86,14 @@ static hermes::vm::ExecutionStatus onJsCallError(hermes::vm::Runtime& runtime, return runtime.setThrownValue(HermesJavaScriptContext::toHermesValue(exception.get())); } +static inline bool handleInterrupt(IJavaScriptContext& jsContext, JSExceptionTracker& exceptionTracker) { + if (VALDI_UNLIKELY(jsContext.interruptRequested()) && jsContext.onInterrupt()) { + exceptionTracker.onError("JavaScript execution terminated"); + return true; + } + return false; +} + hermes::vm::CallResult callTrampoline(void* context, hermes::vm::Runtime& runtime, hermes::vm::NativeArgs args) { @@ -105,8 +113,8 @@ hermes::vm::CallResult callTrampoline(void* context, functionData->function->getReferenceInfo()); callContext.setThisValue(thisRef.get()); - if (functionData->jsContext.interruptRequested()) { - functionData->jsContext.onInterrupt(); + if (handleInterrupt(functionData->jsContext, exceptionTracker)) { + return onJsCallError(runtime, exceptionTracker); } auto result = (*functionData->function)(callContext); @@ -146,8 +154,8 @@ hermes::vm::CallResult callNativeClassInstanceMember( return runtime.raiseTypeError("Native class member called with an incompatible receiver"); } - if (jsContext.interruptRequested()) { - jsContext.onInterrupt(); + if (handleInterrupt(jsContext, exceptionTracker)) { + return onJsCallError(runtime, exceptionTracker); } auto result = functionData->callback(instanceData->getOpaque().get(), callContext); @@ -177,8 +185,8 @@ hermes::vm::CallResult callNativeClassStaticMember( auto thisValue = jsContext.toJSValueRef(args.getThisArg()); callContext.setThisValue(thisValue.get()); - if (jsContext.interruptRequested()) { - jsContext.onInterrupt(); + if (handleInterrupt(jsContext, exceptionTracker)) { + return onJsCallError(runtime, exceptionTracker); } auto result = functionData->callback(functionData->nativeClass->getOpaque().get(), callContext); @@ -230,8 +238,8 @@ hermes::vm::CallResult callNativeClassConstructor(void* auto thisValue = jsContext.toJSValueRef(args.getThisArg()); callContext.setThisValue(thisValue.get()); - if (jsContext.interruptRequested()) { - jsContext.onInterrupt(); + if (handleInterrupt(jsContext, exceptionTracker)) { + return onJsCallError(runtime, exceptionTracker); } auto nativeOpaque = constructor(classData->nativeClass->getOpaque().get(), callContext); diff --git a/valdi/src/valdi/jscore/JSCoreCustomClasses.cpp b/valdi/src/valdi/jscore/JSCoreCustomClasses.cpp index b50cac32..70bc6719 100644 --- a/valdi/src/valdi/jscore/JSCoreCustomClasses.cpp +++ b/valdi/src/valdi/jscore/JSCoreCustomClasses.cpp @@ -30,6 +30,15 @@ static JSObjectRef onJsConstructorError(Valdi::JSExceptionTracker& exceptionTrac return nullptr; } +static inline bool handleInterrupt(Valdi::IJavaScriptContext& jsContext, + Valdi::JSExceptionTracker& exceptionTracker) { + if (VALDI_UNLIKELY(jsContext.interruptRequested()) && jsContext.onInterrupt()) { + exceptionTracker.onError("JavaScript execution terminated"); + return true; + } + return false; +} + JSValueRef callAsFunction(JSContextRef ctx, JSObjectRef function, JSObjectRef thisObject, @@ -53,8 +62,8 @@ JSValueRef callAsFunction(JSContextRef ctx, attachedJsFunctionData->function->getReferenceInfo()); callContext.setThisValue(toValdiJSValue(JSCoreRef(thisObject, kJSTypeObject))); - if (attachedJsFunctionData->jsContext.interruptRequested()) { - attachedJsFunctionData->jsContext.onInterrupt(); + if (handleInterrupt(attachedJsFunctionData->jsContext, exceptionTracker)) { + return onJsCallError(ctx, exceptionTracker, exception); } auto result = (*attachedJsFunctionData->function)(callContext); @@ -128,8 +137,8 @@ static JSObjectRef callNativeClassConstructor(JSContextRef ctx, } return onJsConstructorError(exceptionTracker, exception); } - if (jsContext.interruptRequested()) { - jsContext.onInterrupt(); + if (handleInterrupt(jsContext, exceptionTracker)) { + return onJsConstructorError(exceptionTracker, exception); } auto nativeOpaque = constructor(functionData->nativeClass->getOpaque().get(), callContext); @@ -223,8 +232,8 @@ static JSValueRef callNativeClassInstanceMember(JSContextRef ctx, return onJsCallError(ctx, exceptionTracker, exception); } - if (jsContext.interruptRequested()) { - jsContext.onInterrupt(); + if (handleInterrupt(jsContext, exceptionTracker)) { + return onJsCallError(ctx, exceptionTracker, exception); } auto result = functionData->callback(instanceData->getOpaque().get(), callContext); @@ -257,8 +266,8 @@ static JSValueRef callNativeClassStaticMember(JSContextRef ctx, functionData->referenceInfo); callContext.setThisValue(toValdiJSValue(JSCoreRef(thisObject, kJSTypeObject))); - if (jsContext.interruptRequested()) { - jsContext.onInterrupt(); + if (handleInterrupt(jsContext, exceptionTracker)) { + return onJsCallError(ctx, exceptionTracker, exception); } auto result = functionData->callback(functionData->nativeClass->getOpaque().get(), callContext); diff --git a/valdi/src/valdi/jscore/JavaScriptCoreContext.cpp b/valdi/src/valdi/jscore/JavaScriptCoreContext.cpp index 6a68881c..c4c43f81 100644 --- a/valdi/src/valdi/jscore/JavaScriptCoreContext.cpp +++ b/valdi/src/valdi/jscore/JavaScriptCoreContext.cpp @@ -1330,9 +1330,17 @@ void JavaScriptCoreContext::willEnterVM() { _enterVmCount++; } +void JavaScriptCoreContext::requestExecutionTermination() { + IJavaScriptContext::requestExecutionTermination(); +} + void JavaScriptCoreContext::willExitVM(Valdi::JSExceptionTracker& exceptionTracker) { auto enterVMCount = --_enterVmCount; if (enterVMCount == 0) { + if (executionTerminationRequested()) { + return; + } + while (!_microtasks.empty()) { auto microtask = _microtasks.front(); _microtasks.pop_front(); diff --git a/valdi/src/valdi/jscore/JavaScriptCoreContext.hpp b/valdi/src/valdi/jscore/JavaScriptCoreContext.hpp index 435c8fff..dc2e5902 100644 --- a/valdi/src/valdi/jscore/JavaScriptCoreContext.hpp +++ b/valdi/src/valdi/jscore/JavaScriptCoreContext.hpp @@ -175,6 +175,9 @@ class JavaScriptCoreContext : public Valdi::IJavaScriptContext { void willEnterVM() override; void willExitVM(Valdi::JSExceptionTracker& exceptionTracker) override; + void requestExecutionTermination() override; + + const Valdi::JSPropertyName& getPrototypePropertyName() const; const Valdi::JSPropertyName& getPrototypePropertyName() const; diff --git a/valdi/src/valdi/quickjs/QuickJSJavaScriptContext.cpp b/valdi/src/valdi/quickjs/QuickJSJavaScriptContext.cpp index 36f1b147..1f844ed6 100644 --- a/valdi/src/valdi/quickjs/QuickJSJavaScriptContext.cpp +++ b/valdi/src/valdi/quickjs/QuickJSJavaScriptContext.cpp @@ -79,7 +79,7 @@ constexpr double kMaxStackSizeRatio = 0.75; static int handleInterrupt(JSRuntime* rt, void* opaque) { auto& jsContext = *reinterpret_cast(opaque); if (jsContext.interruptRequested()) { - jsContext.onInterrupt(); + return jsContext.onInterrupt() ? 1 : 0; } return 0; } @@ -1462,6 +1462,10 @@ void QuickJSJavaScriptContext::willEnterVM() { } } +void QuickJSJavaScriptContext::requestExecutionTermination() { + IJavaScriptContext::requestExecutionTermination(); +} + struct StackLimits { void* startFrameAddress; size_t maxStackSize; @@ -1534,6 +1538,10 @@ void QuickJSJavaScriptContext::willExitVM(Valdi::JSExceptionTracker& exceptionTr SC_ASSERT(_enterVmCount > 0); --_enterVmCount; if (_enterVmCount == 0) { + if (executionTerminationRequested()) { + return; + } + JSContext* context = nullptr; for (;;) { diff --git a/valdi/src/valdi/quickjs/QuickJSJavaScriptContext.hpp b/valdi/src/valdi/quickjs/QuickJSJavaScriptContext.hpp index 55d343f2..fc280e90 100644 --- a/valdi/src/valdi/quickjs/QuickJSJavaScriptContext.hpp +++ b/valdi/src/valdi/quickjs/QuickJSJavaScriptContext.hpp @@ -199,6 +199,7 @@ class QuickJSJavaScriptContext : public Valdi::IJavaScriptContext { void willEnterVM() override; void willExitVM(Valdi::JSExceptionTracker& exceptionTracker) override; + void requestExecutionTermination() override; protected: Valdi::JSValueRef onNewBool(bool boolean) override; diff --git a/valdi/src/valdi/quickjs/QuickJSUtils.cpp b/valdi/src/valdi/quickjs/QuickJSUtils.cpp index a0c81263..91d4ac15 100644 --- a/valdi/src/valdi/quickjs/QuickJSUtils.cpp +++ b/valdi/src/valdi/quickjs/QuickJSUtils.cpp @@ -24,6 +24,15 @@ static JSValue onJsCallError(JSContext* context, Valdi::JSExceptionTracker& exce return JS_Throw(context, JS_DupValue(context, fromValdiJSValue(exception.get()))); } +static inline bool handleInterrupt(Valdi::IJavaScriptContext& jsContext, + Valdi::JSExceptionTracker& exceptionTracker) { + if (VALDI_UNLIKELY(jsContext.interruptRequested()) && jsContext.onInterrupt()) { + exceptionTracker.onError("JavaScript execution terminated"); + return true; + } + return false; +} + NativeClassFunctionData::NativeClassFunctionData(const Valdi::Ref& nativeClass, const Valdi::StringBox& name) : nativeClass(nativeClass), @@ -55,8 +64,8 @@ static JSValue callNativeClassInstanceMember(JSContext* context, return JS_ThrowTypeError(context, "Native class member called with an incompatible receiver"); } - if (valdiJsContext.interruptRequested()) { - valdiJsContext.onInterrupt(); + if (handleInterrupt(valdiJsContext, exceptionTracker)) { + return onJsCallError(context, exceptionTracker); } auto result = functionData->callback(instanceData->getOpaque().get(), callContext); @@ -86,8 +95,8 @@ static JSValue callNativeClassStaticMember(JSContext* context, valdiJsContext, arguments, static_cast(argc), exceptionTracker, functionData->referenceInfo); callContext.setThisValue(toValdiJSValue(thisValue)); - if (valdiJsContext.interruptRequested()) { - valdiJsContext.onInterrupt(); + if (handleInterrupt(valdiJsContext, exceptionTracker)) { + return onJsCallError(context, exceptionTracker); } auto result = functionData->callback(functionData->nativeClass->getOpaque().get(), callContext); @@ -134,8 +143,9 @@ static JSValue callNativeClassConstructor( } callContext.setThisValue(toValdiJSValue(object)); - if (valdiJsContext.interruptRequested()) { - valdiJsContext.onInterrupt(); + if (handleInterrupt(valdiJsContext, exceptionTracker)) { + JS_FreeValue(context, object); + return onJsCallError(context, exceptionTracker); } auto nativeOpaque = constructor(nativeClass->getOpaque().get(), callContext); @@ -170,8 +180,8 @@ JSValue jsCall( valdiJsContext, arguments, static_cast(argc), exceptionTracker, function->getReferenceInfo()); callContext.setThisValue(toValdiJSValue(thisValue)); - if (valdiJsContext.interruptRequested()) { - valdiJsContext.onInterrupt(); + if (handleInterrupt(valdiJsContext, exceptionTracker)) { + return onJsCallError(context, exceptionTracker); } auto result = (*function)(callContext); diff --git a/valdi/src/valdi/runtime/Interfaces/IJavaScriptContext.cpp b/valdi/src/valdi/runtime/Interfaces/IJavaScriptContext.cpp index 71c449e1..2401eabf 100644 --- a/valdi/src/valdi/runtime/Interfaces/IJavaScriptContext.cpp +++ b/valdi/src/valdi/runtime/Interfaces/IJavaScriptContext.cpp @@ -423,21 +423,36 @@ bool IJavaScriptContext::utf16Disabled() const { return _utf16Disabled; } -// We don't care about pure thread safety correctness of the -// interrupt boolean, it needs to be as low overhead as possible +// We don't care about pure thread safety correctness of the interrupt booleans. +// They need to be as low overhead as possible because they are polled from hot +// native call and VM interrupt paths. __attribute__((no_sanitize("thread"))) void IJavaScriptContext::requestInterrupt() { _interruptRequested = true; } -__attribute__((no_sanitize("thread"))) void IJavaScriptContext::onInterrupt() { - if (_listener != nullptr) { - _listener->onInterrupt(*this); +__attribute__((no_sanitize("thread"))) void IJavaScriptContext::requestExecutionTermination() { + // Unlike the diagnostic interrupt, this is monotonic for the lifetime of + // the context. Keeping it sticky also makes native-call checkpoints useful + // for engines which cannot interrupt pure JavaScript execution. + _executionTerminationRequested = true; +} + +__attribute__((no_sanitize("thread"))) bool IJavaScriptContext::onInterrupt() { + if (_interruptRequested) { + _interruptRequested = false; + if (_listener != nullptr) { + _listener->onInterrupt(*this); + } } - _interruptRequested = false; + return _executionTerminationRequested; } __attribute__((no_sanitize("thread"))) bool IJavaScriptContext::interruptRequested() const { - return _interruptRequested; + return _interruptRequested || _executionTerminationRequested; +} + +__attribute__((no_sanitize("thread"))) bool IJavaScriptContext::executionTerminationRequested() const { + return _executionTerminationRequested; } JSValueID IJavaScriptContext::stashJSValue(JSValueRef&& valueRef) { diff --git a/valdi/src/valdi/runtime/Interfaces/IJavaScriptContext.hpp b/valdi/src/valdi/runtime/Interfaces/IJavaScriptContext.hpp index 84dd6e9b..fb543d96 100644 --- a/valdi/src/valdi/runtime/Interfaces/IJavaScriptContext.hpp +++ b/valdi/src/valdi/runtime/Interfaces/IJavaScriptContext.hpp @@ -358,9 +358,11 @@ class IJavaScriptContext : public SharedPtrRefCountable, public snap::NonCopyabl bool utf16Disabled() const; void requestInterrupt(); - void onInterrupt(); + virtual void requestExecutionTermination(); + bool onInterrupt(); bool interruptRequested() const; + bool executionTerminationRequested() const; virtual void willEnterVM(); @@ -440,6 +442,7 @@ class IJavaScriptContext : public SharedPtrRefCountable, public snap::NonCopyabl JSPropertyNameRef _exportModePropertyName; bool _utf16Disabled = false; bool _interruptRequested = false; + bool _executionTerminationRequested = false; bool _tearingDown = false; std::unique_ptr _valueMarshaller; diff --git a/valdi/src/valdi/runtime/JavaScript/JavaScriptMessagePort.cpp b/valdi/src/valdi/runtime/JavaScript/JavaScriptMessagePort.cpp new file mode 100644 index 00000000..7e523dbb --- /dev/null +++ b/valdi/src/valdi/runtime/JavaScript/JavaScriptMessagePort.cpp @@ -0,0 +1,511 @@ +#include "valdi/runtime/JavaScript/JavaScriptMessagePort.hpp" + +#include "valdi/runtime/JavaScript/JSNativeClassBinder.hpp" +#include "valdi/runtime/JavaScript/JavaScriptFunctionCallContext.hpp" +#include "valdi/runtime/JavaScript/JavaScriptRuntime.hpp" +#include "valdi/runtime/JavaScript/JavaScriptUtils.hpp" +#include "valdi_core/cpp/Utils/ValueArray.hpp" + +#include + +namespace Valdi { + +JavaScriptMessage::JavaScriptMessage(Value data, + std::vector> transferredPortSources, + std::vector> transferredPorts) + : _data(std::move(data)), + _transferredPortSources(std::move(transferredPortSources)), + _transferredPorts(std::move(transferredPorts)) {} + +const Value& JavaScriptMessage::getData() const { + return _data; +} + +const std::vector>& JavaScriptMessage::getTransferredPortSources() const { + return _transferredPortSources; +} + +const std::vector>& JavaScriptMessage::getTransferredPorts() const { + return _transferredPorts; +} + +void JavaScriptMessagePortEndpoint::setPeer(const Ref& peer) { + std::lock_guard lock(_mutex); + _peer = peer; +} + +void JavaScriptMessagePortEndpoint::attach(const Ref& handle, + const Ref& runtime) { + std::lock_guard lock(_mutex); + SC_ASSERT(!_closed); + SC_ASSERT(_handle.expired()); + auto ownerRuntime = _ownerRuntime.lock(); + SC_ASSERT(ownerRuntime == nullptr || ownerRuntime.get() == runtime.get()); + _ownerRuntime = runtime; + _handle = handle; +} + +Result JavaScriptMessagePortEndpoint::validateTransfer(const JavaScriptMessagePort& handle) const { + std::lock_guard lock(_mutex); + auto currentHandle = _handle.lock(); + if (_closed || currentHandle == nullptr || currentHandle.get() != &handle) { + return Error("MessagePort in transfer list is already detached"); + } + return Void(); +} + +void JavaScriptMessagePortEndpoint::transfer(const JavaScriptMessagePort& handle, + const Ref& runtime) { + std::lock_guard lock(_mutex); + auto currentHandle = _handle.lock(); + SC_ASSERT(!_closed && currentHandle != nullptr && currentHandle.get() == &handle); + _generation++; + _scheduledGeneration.reset(); + _started = false; + _handle.reset(); + _ownerRuntime = runtime; +} + +Ref JavaScriptMessagePortEndpoint::getOwnerRuntime() const { + std::lock_guard lock(_mutex); + return Ref(_ownerRuntime.lock()); +} + +Ref JavaScriptMessagePortEndpoint::getPeer() const { + std::lock_guard lock(_mutex); + return Ref(_peer.lock()); +} + +void JavaScriptMessagePortEndpoint::start(const JavaScriptMessagePort& handle) { + std::unique_lock lock(_mutex); + auto currentHandle = _handle.lock(); + if (_closed || currentHandle == nullptr || currentHandle.get() != &handle) { + return; + } + _started = true; + scheduleNextMessage(lock); +} + +void JavaScriptMessagePortEndpoint::close(const JavaScriptMessagePort& handle) { + std::lock_guard lock(_mutex); + auto currentHandle = _handle.lock(); + if (_closed || currentHandle == nullptr || currentHandle.get() != &handle) { + return; + } + _closed = true; + _generation++; + _scheduledGeneration.reset(); + _handle.reset(); + _ownerRuntime.reset(); + _messages.clear(); +} + +void JavaScriptMessagePortEndpoint::enqueue(const Ref& message) { + for (const auto& transferredPort : message->getTransferredPorts()) { + if (transferredPort.get() == this) { + // Transferring the destination port makes the message undeliverable. + return; + } + } + + std::unique_lock lock(_mutex); + if (_closed) { + return; + } + _messages.emplace_back(message); + scheduleNextMessage(lock); +} + +void JavaScriptMessagePortEndpoint::scheduleNextMessage(std::unique_lock& lock) { + if (_closed || !_started || _messages.empty() || _handle.expired() || _scheduledGeneration.has_value()) { + return; + } + + auto runtime = Ref(_ownerRuntime.lock()); + if (runtime == nullptr) { + return; + } + + auto generation = _generation; + _scheduledGeneration = generation; + auto self = strongSmallRef(this); + lock.unlock(); + runtime->dispatchOnJsThread( + nullptr, JavaScriptTaskScheduleTypeAlwaysAsync, 0, [self, generation](JavaScriptEntryParameters& entry) { + self->dispatchNextMessage(entry, generation); + }); +} + +void JavaScriptMessagePortEndpoint::dispatchNextMessage(JavaScriptEntryParameters& entry, uint64_t generation) { + Ref handle; + Ref message; + { + std::lock_guard lock(_mutex); + if (_closed || generation != _generation || _scheduledGeneration != generation || !_started || + _messages.empty()) { + return; + } + _scheduledGeneration.reset(); + handle = Ref(_handle.lock()); + if (handle == nullptr) { + return; + } + message = std::move(_messages.front()); + _messages.pop_front(); + } + + handle->dispatchMessage(entry, message); + + std::unique_lock lock(_mutex); + scheduleNextMessage(lock); +} + +VALDI_CLASS_IMPL(JavaScriptMessagePort); + +JavaScriptMessagePort::JavaScriptMessagePort(Ref endpoint) + : _endpoint(std::move(endpoint)) {} + +JavaScriptMessagePort::~JavaScriptMessagePort() { + close(); +} + +bool JavaScriptMessagePort::isActive() const { + std::lock_guard lock(_mutex); + return _active; +} + +Result JavaScriptMessagePort::validateTransfer(const JavaScriptMessagePort* sourcePort) const { + { + std::lock_guard lock(_mutex); + if (!_active) { + return Error("MessagePort in transfer list is already detached"); + } + if (sourcePort == this) { + return Error("Transfer list contains source MessagePort"); + } + } + return _endpoint->validateTransfer(*this); +} + +Ref JavaScriptMessagePort::transfer(const Ref& runtime) { + { + std::lock_guard lock(_mutex); + SC_ASSERT(_active); + _active = false; + _onMessage.reset(); + } + _endpoint->transfer(*this, runtime); + return _endpoint; +} + +Ref JavaScriptMessagePort::getPeerRuntime() const { + auto peer = _endpoint->getPeer(); + return peer != nullptr ? peer->getOwnerRuntime() : nullptr; +} + +void JavaScriptMessagePort::postMessage(const Ref& message) { + auto peer = _endpoint->getPeer(); + if (peer != nullptr) { + peer->enqueue(message); + } +} + +void JavaScriptMessagePort::start() { + if (isActive()) { + _endpoint->start(*this); + } +} + +void JavaScriptMessagePort::close() { + { + std::lock_guard lock(_mutex); + if (!_active) { + return; + } + _active = false; + _onMessage.reset(); + } + _endpoint->close(*this); +} + +void JavaScriptMessagePort::setOnMessage(Shared callback) { + const auto shouldStart = callback != nullptr; + { + std::lock_guard lock(_mutex); + if (!_active) { + return; + } + _onMessage = std::move(callback); + } + if (shouldStart) { + _endpoint->start(*this); + } +} + +Shared JavaScriptMessagePort::getOnMessage() const { + std::lock_guard lock(_mutex); + return _onMessage; +} + +void JavaScriptMessagePort::dispatchMessage(JavaScriptEntryParameters& entry, const Ref& message) { + auto callback = getOnMessage(); + if (callback == nullptr) { + return; + } + auto handler = callback->getJsValue(entry.jsContext, entry.exceptionTracker); + if (!entry.exceptionTracker) { + return; + } + message->callHandler(entry, handler); +} + +Result> JavaScriptMessage::make(const Value& data, + const Value& transfer, + const Ref& targetRuntime, + const JavaScriptMessagePort* sourcePort) { + std::vector> ports; + if (!transfer.isUndefined()) { + if (!transfer.isArray()) { + return Error("MessagePort transfer list must be an array"); + } + const auto* values = transfer.getArray(); + ports.reserve(values->size()); + std::unordered_set uniquePorts; + for (const auto& value : *values) { + if (!value.isValdiObject()) { + return Error("Transfer list contains an unsupported value"); + } + auto port = castOrNull(value.getValdiObject()); + if (port == nullptr) { + return Error("Only MessagePort values can be transferred"); + } + if (!uniquePorts.insert(port.get()).second) { + return Error("Transfer list contains duplicate MessagePort"); + } + auto validation = port->validateTransfer(sourcePort); + if (!validation) { + return validation.moveError(); + } + ports.emplace_back(std::move(port)); + } + } + + std::vector> transferredPortSources; + transferredPortSources.reserve(ports.size()); + std::vector> transferredPorts; + transferredPorts.reserve(ports.size()); + for (const auto& port : ports) { + transferredPortSources.emplace_back(port); + transferredPorts.emplace_back(port->transfer(targetRuntime)); + } + return makeShared(data, std::move(transferredPortSources), std::move(transferredPorts)); +} + +JSValueRef JavaScriptMessagePort::makeClass(IJavaScriptContext& context, JSExceptionTracker& exceptionTracker) { + auto definition = JSNativeClassBinder("MessagePort") + .bindMethod<&JavaScriptMessagePort::postMessageFromJavaScript>("postMessage") + .bindMethod<&JavaScriptMessagePort::startFromJavaScript>("start") + .bindMethod<&JavaScriptMessagePort::closeFromJavaScript>("close") + .bindAccessor<&JavaScriptMessagePort::getOnMessageForJavaScript, + &JavaScriptMessagePort::setOnMessageForJavaScript>("onmessage", true, true) + .extractClassDefinition(); + return context.newNativeClass(nullptr, definition, exceptionTracker); +} + +JSValueRef JavaScriptMessagePort::make(IJavaScriptContext& context, + JSExceptionTracker& exceptionTracker, + const Ref& endpoint) { + auto* runtime = dynamic_cast(context.getTaskScheduler()); + SC_ASSERT(runtime != nullptr); + + auto port = makeShared(endpoint); + endpoint->attach(port, strongSmallRef(runtime)); + auto portClass = context.getPropertyFromGlobalObjectCached(STRING_LITERAL("MessagePort"), exceptionTracker); + if (!exceptionTracker) { + return context.newUndefined(); + } + return context.newObjectFromNativeClass(port, portClass.get(), exceptionTracker); +} + +JSValueRef JavaScriptMessagePort::postMessageFromJavaScript(Value data, + Value transfer, + JSFunctionNativeCallContext& callContext) { + if (!isActive()) { + return callContext.getContext().newUndefined(); + } + + auto targetRuntime = getPeerRuntime(); + if (targetRuntime == nullptr) { + return callContext.getContext().newUndefined(); + } + + auto message = JavaScriptMessage::make(data, transfer, targetRuntime, this); + if (!message) { + return callContext.throwError(message.moveError()); + } + postMessage(message.moveValue()); + return callContext.getContext().newUndefined(); +} + +void JavaScriptMessagePort::startFromJavaScript(JSFunctionNativeCallContext& /*callContext*/) { + start(); +} + +void JavaScriptMessagePort::closeFromJavaScript(JSFunctionNativeCallContext& /*callContext*/) { + close(); +} + +JSValueRef JavaScriptMessagePort::getOnMessageForJavaScript(JSFunctionNativeCallContext& callContext) { + auto callback = getOnMessage(); + if (callback == nullptr) { + return callContext.getContext().newNull(); + } + + auto value = callback->getJsValue(callContext.getContext(), callContext.getExceptionTracker()); + if (!callContext.getExceptionTracker()) { + return callContext.getContext().newUndefined(); + } + return JSValueRef::makeRetained(callContext.getContext(), value); +} + +void JavaScriptMessagePort::setOnMessageForJavaScript(JSValue value, JSFunctionNativeCallContext& callContext) { + if (!callContext.getContext().isValueFunction(value)) { + setOnMessage(nullptr); + return; + } + + auto callback = + JSValueRefHolder::makeRetainedCallback(callContext.getContext(), + value, + ReferenceInfoBuilder().withProperty(STRING_LITERAL("onmessage")), + callContext.getExceptionTracker()); + if (!callContext.getExceptionTracker()) { + return; + } + setOnMessage(std::move(callback)); +} + +JSValueRef JavaScriptMessageChannel::makeClass(IJavaScriptContext& context, JSExceptionTracker& exceptionTracker) { + auto definition = JSNativeClassBinder("MessageChannel").extractClassDefinition(); + definition.setConstructor(&JavaScriptMessageChannel::construct); + return context.newNativeClass(nullptr, definition, exceptionTracker); +} + +Ref JavaScriptMessageChannel::construct(RefCountable* /*classOpaque*/, + JSFunctionNativeCallContext& callContext) noexcept { + auto firstEndpoint = makeShared(); + auto secondEndpoint = makeShared(); + firstEndpoint->setPeer(secondEndpoint); + secondEndpoint->setPeer(firstEndpoint); + + auto firstPort = + JavaScriptMessagePort::make(callContext.getContext(), callContext.getExceptionTracker(), firstEndpoint); + if (!callContext.getExceptionTracker()) { + return nullptr; + } + auto secondPort = + JavaScriptMessagePort::make(callContext.getContext(), callContext.getExceptionTracker(), secondEndpoint); + if (!callContext.getExceptionTracker()) { + return nullptr; + } + + auto& context = callContext.getContext(); + auto& exceptionTracker = callContext.getExceptionTracker(); + context.setObjectProperty(callContext.getThisValue(), "port1", firstPort.get(), exceptionTracker); + context.setObjectProperty(callContext.getThisValue(), "port2", secondPort.get(), exceptionTracker); + if (!exceptionTracker) { + return nullptr; + } + return makeShared(); +} + +class TransferredPortJSValueResolver final : public JSValueForNativeObjectResolver { +public: + TransferredPortJSValueResolver(const JavaScriptMessage& message, const std::vector& transferredPorts) + : _message(message), _transferredPorts(transferredPorts) {} + + std::optional getJSValueForNativeObject(RefCountable* object) final { + const auto& transferredPortSources = _message.getTransferredPortSources(); + for (size_t i = 0; i < transferredPortSources.size(); i++) { + if (object == transferredPortSources[i].get()) { + return _transferredPorts[i].get(); + } + } + return std::nullopt; + } + +private: + const JavaScriptMessage& _message; + const std::vector& _transferredPorts; +}; + +static JSValueRef makeJavaScriptMessageEvent(JavaScriptEntryParameters& entry, const JavaScriptMessage& message) { + auto ports = entry.jsContext.newArray(message.getTransferredPorts().size(), entry.exceptionTracker); + if (!entry.exceptionTracker) { + return entry.jsContext.newUndefined(); + } + std::vector transferredPorts; + transferredPorts.reserve(message.getTransferredPorts().size()); + size_t index = 0; + for (const auto& endpoint : message.getTransferredPorts()) { + auto port = JavaScriptMessagePort::make(entry.jsContext, entry.exceptionTracker, endpoint); + if (!entry.exceptionTracker) { + return entry.jsContext.newUndefined(); + } + entry.jsContext.setObjectPropertyIndex(ports.get(), index++, port.get(), entry.exceptionTracker); + if (!entry.exceptionTracker) { + return entry.jsContext.newUndefined(); + } + transferredPorts.emplace_back(std::move(port)); + } + + TransferredPortJSValueResolver nativeObjectResolver(message, transferredPorts); + auto data = valueToJSValue(entry.jsContext, + message.getData(), + &nativeObjectResolver, + ReferenceInfoBuilder().withProperty(STRING_LITERAL("data")), + entry.exceptionTracker); + if (!entry.exceptionTracker) { + return entry.jsContext.newUndefined(); + } + + auto event = entry.jsContext.newObject(entry.exceptionTracker); + entry.jsContext.setObjectProperty(event.get(), "data", data.get(), entry.exceptionTracker); + entry.jsContext.setObjectProperty(event.get(), "ports", ports.get(), entry.exceptionTracker); + return event; +} + +void JavaScriptMessage::callHandler(JavaScriptEntryParameters& entry, const JSValue& handler) { + auto event = makeJavaScriptMessageEvent(entry, *this); + if (!entry.exceptionTracker) { + return; + } + JSValueRef parameters[] = {std::move(event)}; + JSFunctionCallContext callContext(entry.jsContext, parameters, 1, entry.exceptionTracker); + entry.jsContext.callObjectAsFunction(handler, callContext); +} + +void JavaScriptMessage::dispatchHandler(const Shared& handler, Function&& shouldDispatch) { + if (handler == nullptr) { + return; + } + auto scheduler = handler->getTaskScheduler(); + if (scheduler == nullptr) { + return; + } + auto self = strongSmallRef(this); + scheduler->dispatchOnJsThreadAsync( + handler->getContext(), + [handler, self, shouldDispatch = std::move(shouldDispatch)](JavaScriptEntryParameters& entry) { + if (!shouldDispatch()) { + return; + } + auto function = handler->getJsValue(entry.jsContext, entry.exceptionTracker); + if (!entry.exceptionTracker) { + return; + } + self->callHandler(entry, function); + }); +} + +} // namespace Valdi diff --git a/valdi/src/valdi/runtime/JavaScript/JavaScriptMessagePort.hpp b/valdi/src/valdi/runtime/JavaScript/JavaScriptMessagePort.hpp new file mode 100644 index 00000000..b43d7cc7 --- /dev/null +++ b/valdi/src/valdi/runtime/JavaScript/JavaScriptMessagePort.hpp @@ -0,0 +1,122 @@ +#pragma once + +#include "valdi/runtime/JavaScript/JSValueRefHolder.hpp" +#include "valdi/runtime/JavaScript/JavaScriptTaskScheduler.hpp" +#include "valdi_core/cpp/Utils/Mutex.hpp" +#include "valdi_core/cpp/Utils/Result.hpp" +#include "valdi_core/cpp/Utils/ValdiObject.hpp" +#include "valdi_core/cpp/Utils/Value.hpp" + +#include +#include +#include + +namespace Valdi { + +class JavaScriptRuntime; +class JSFunctionNativeCallContext; +class JavaScriptMessagePort; +class JavaScriptMessagePortEndpoint; + +class JavaScriptMessage final : public SharedPtrRefCountable { +public: + static Result> make(const Value& data, + const Value& transfer, + const Ref& targetRuntime, + const JavaScriptMessagePort* sourcePort); + + JavaScriptMessage(Value data, + std::vector> transferredPortSources, + std::vector> transferredPorts); + + const Value& getData() const; + const std::vector>& getTransferredPortSources() const; + const std::vector>& getTransferredPorts() const; + + void callHandler(JavaScriptEntryParameters& entry, const JSValue& handler); + void dispatchHandler(const Shared& handler, Function&& shouldDispatch); + +private: + Value _data; + std::vector> _transferredPortSources; + std::vector> _transferredPorts; +}; + +class JavaScriptMessagePortEndpoint final : public SharedPtrRefCountable { +public: + void setPeer(const Ref& peer); + + void attach(const Ref& handle, const Ref& runtime); + Result validateTransfer(const JavaScriptMessagePort& handle) const; + void transfer(const JavaScriptMessagePort& handle, const Ref& runtime); + + Ref getOwnerRuntime() const; + Ref getPeer() const; + + void start(const JavaScriptMessagePort& handle); + void close(const JavaScriptMessagePort& handle); + void enqueue(const Ref& message); + +private: + mutable Mutex _mutex; + Weak _peer; + Weak _ownerRuntime; + Weak _handle; + std::deque> _messages; + std::optional _scheduledGeneration; + uint64_t _generation = 0; + bool _started = false; + bool _closed = false; + + void scheduleNextMessage(std::unique_lock& lock); + void dispatchNextMessage(JavaScriptEntryParameters& entry, uint64_t generation); +}; + +class JavaScriptMessagePort final : public ValdiObject { +public: + VALDI_CLASS_HEADER(JavaScriptMessagePort); + + static JSValueRef makeClass(IJavaScriptContext& context, JSExceptionTracker& exceptionTracker); + + static JSValueRef make(IJavaScriptContext& context, + JSExceptionTracker& exceptionTracker, + const Ref& endpoint); + + explicit JavaScriptMessagePort(Ref endpoint); + ~JavaScriptMessagePort() override; + + bool isActive() const; + Result validateTransfer(const JavaScriptMessagePort* sourcePort) const; + Ref transfer(const Ref& runtime); + + Ref getPeerRuntime() const; + void postMessage(const Ref& message); + void start(); + void close(); + + void setOnMessage(Shared callback); + Shared getOnMessage() const; + void dispatchMessage(JavaScriptEntryParameters& entry, const Ref& message); + +private: + Ref _endpoint; + mutable Mutex _mutex; + Shared _onMessage; + bool _active = true; + + JSValueRef postMessageFromJavaScript(Value data, Value transfer, JSFunctionNativeCallContext& callContext); + void startFromJavaScript(JSFunctionNativeCallContext& callContext); + void closeFromJavaScript(JSFunctionNativeCallContext& callContext); + JSValueRef getOnMessageForJavaScript(JSFunctionNativeCallContext& callContext); + void setOnMessageForJavaScript(JSValue value, JSFunctionNativeCallContext& callContext); +}; + +class JavaScriptMessageChannel final : public SimpleRefCountable { +public: + static JSValueRef makeClass(IJavaScriptContext& context, JSExceptionTracker& exceptionTracker); + +private: + static Ref construct(RefCountable* classOpaque, JSFunctionNativeCallContext& callContext) noexcept; +}; + +} // namespace Valdi diff --git a/valdi/src/valdi/runtime/JavaScript/JavaScriptRuntime.cpp b/valdi/src/valdi/runtime/JavaScript/JavaScriptRuntime.cpp index 0b1acfeb..c99686a2 100644 --- a/valdi/src/valdi/runtime/JavaScript/JavaScriptRuntime.cpp +++ b/valdi/src/valdi/runtime/JavaScript/JavaScriptRuntime.cpp @@ -21,6 +21,7 @@ #include "valdi/runtime/JavaScript/JavaScriptContextEntryPoint.hpp" #include "valdi/runtime/JavaScript/JavaScriptErrorStackTrace.hpp" #include "valdi/runtime/JavaScript/JavaScriptFunctionCallContext.hpp" +#include "valdi/runtime/JavaScript/JavaScriptMessagePort.hpp" #include "valdi/runtime/JavaScript/JavaScriptModuleContainer.hpp" #include "valdi/runtime/JavaScript/JavaScriptUtils.hpp" #include "valdi/runtime/JavaScript/JavaScriptValueMarshaller.hpp" @@ -405,6 +406,43 @@ void JavaScriptRuntime::partialTeardown() { teardown(false); } +void JavaScriptRuntime::requestFullTeardown() { + auto wasDisposed = _isDisposed.exchange(true); + if (wasDisposed) { + return; + } + + // Just in case postInit() was never called + _initLock.leaveIfNotCompleted(); + + _dispatchQueue->async([self = strongSmallRef(this)]() { + self->teardownOnJsThread(true); + }); +} + +void JavaScriptRuntime::requestExecutionTermination() { + Ref jsContext; + { + std::lock_guard lock(_mutex); + if (_isDisposed) { + return; + } + jsContext = _javaScriptContext; + } + + if (jsContext == nullptr) { + return; + } + + jsContext->requestExecutionTermination(); + + if (!_dispatchQueue->isCurrent()) { + // Ensure the last reference obtained from another thread is released on + // the JavaScript queue before the teardown task destroys the context. + _dispatchQueue->async([jsContext = std::move(jsContext)]() {}); + } +} + void JavaScriptRuntime::teardown(bool destroyContext) { auto wasDisposed = _isDisposed.exchange(true); if (wasDisposed) { @@ -422,49 +460,55 @@ void JavaScriptRuntime::teardown(bool destroyContext) { } if (destroyContext) { - _dispatchQueue->sync([&]() { - if (_anrDetector != nullptr) { - _anrDetector->removeTaskScheduler(this); - } - // Prevent further dispatches to run - setListener(nullptr, {}); - _dispatchQueue->fullTeardown(); + _dispatchQueue->safeSync([&]() { + teardownOnJsThread(true); + }); + } else { + teardownOnJsThread(false); + } +} - if (!destroyContext) { - return; - } +void JavaScriptRuntime::teardownOnJsThread(bool destroyContext) { + if (_anrDetector != nullptr) { + _anrDetector->removeTaskScheduler(this); + } - // Bridged objects should be deallocated immediately - auto nonDeferredPool = Valdi::RefCountableAutoreleasePool::makeNonDeferred(); - - _globalContext->releaseDisposables(); - _contextManager.destroyContext(_globalContext); - - _modules.clear(); - _daemonClients.clear(); - _moduleLoader = JSValueRef(); - _symbolicateFunction = Result(); - _onDaemonClientEventFunction = Result(); - _uncaughtExceptionHandler = nullptr; - _unhandledRejectionHandler = nullptr; - if (_contextHandler != nullptr) { - _contextHandler->clear(); - _contextHandler = nullptr; - } - _runtimeDeserializers = nullptr; - _propertyNameIndex.setContext(nullptr); - auto weakJavaScriptContext = weakRef(_javaScriptContext.get()); - _javaScriptContext = nullptr; + _running = false; + setListener(nullptr, {}); + _dispatchQueue->fullTeardown(); - SC_ASSERT(weakJavaScriptContext.expired()); - }); - } else { - if (_anrDetector != nullptr) { - _anrDetector->removeTaskScheduler(this); - } - _dispatchQueue->fullTeardown(); - setListener(nullptr, {}); + if (!destroyContext) { + return; + } + + // Bridged objects should be deallocated immediately + auto nonDeferredPool = Valdi::RefCountableAutoreleasePool::makeNonDeferred(); + + _globalContext->releaseDisposables(); + _contextManager.destroyContext(_globalContext); + + _modules.clear(); + _daemonClients.clear(); + _moduleLoader = JSValueRef(); + _symbolicateFunction = Result(); + _onDaemonClientEventFunction = Result(); + _uncaughtExceptionHandler = nullptr; + _unhandledRejectionHandler = nullptr; + if (_contextHandler != nullptr) { + _contextHandler->clear(); + _contextHandler = nullptr; } + _runtimeDeserializers = nullptr; + _propertyNameIndex.setContext(nullptr); + + Weak weakJavaScriptContext; + { + std::lock_guard lock(_mutex); + weakJavaScriptContext = weakRef(_javaScriptContext.get()); + _javaScriptContext = nullptr; + } + + SC_ASSERT(weakJavaScriptContext.expired()); } void JavaScriptRuntime::setThreadQoS(ThreadQoSClass threadQoS) { @@ -545,7 +589,10 @@ Result JavaScriptRuntime::initializeContext() { } if (exceptionTracker) { - _javaScriptContext = std::move(jsContext); + { + std::lock_guard lock(_mutex); + _javaScriptContext = std::move(jsContext); + } _runtimeDeserializers = std::move(runtimeDeserializers); _propertyNameIndex.setContext(_javaScriptContext.get()); return Void(); @@ -2213,7 +2260,8 @@ JSValueRef JavaScriptRuntime::runtimeCreateWorker(JSFunctionNativeCallContext& c for (const auto& typeConverter : _typeConverters) { workerRuntime->registerTypeConverter(typeConverter.typeName, typeConverter.functionPath); } - auto worker = makeShared(workerRuntime, callContext.getParameterAsString(0)); + auto worker = + makeShared(strongSmallRef(this), workerRuntime, callContext.getParameterAsString(0)); worker->postInit(); CHECK_CALL_CONTEXT(callContext); // worker->init(); // call outside of ctor so that shared_from_this() is available @@ -2264,18 +2312,34 @@ Ref thisFromCallContext(JSFunctionNativeCallContext& callContext) { JSValueRef JavaScriptRuntime::workerSetOnMessage(JSFunctionNativeCallContext& callContext) { auto worker = thisFromCallContext(callContext); if (worker != nullptr) { - worker->setHostOnMessage(callContext.getParameterAsFunction(0)); + auto callbackValue = callContext.getParameter(0); + if (!callContext.getContext().isValueFunction(callbackValue)) { + return callContext.throwError(Error("Expecting Worker onmessage function")); + } + auto callback = + JSValueRefHolder::makeRetainedCallback(callContext.getContext(), + callbackValue, + ReferenceInfoBuilder().withProperty(STRING_LITERAL("onmessage")), + callContext.getExceptionTracker()); CHECK_CALL_CONTEXT(callContext); + worker->setHostOnMessage(std::move(callback)); } return callContext.getContext().newUndefined(); } -// worker.postMessage(any) +// worker.postMessage(any, MessagePort[] | undefined) JSValueRef JavaScriptRuntime::workerPostMessage(JSFunctionNativeCallContext& callContext) { auto worker = thisFromCallContext(callContext); if (worker != nullptr) { - worker->postMessage(callContext.getParameterAsValue(0)); + auto data = callContext.getParameterAsValue(0); + CHECK_CALL_CONTEXT(callContext); + auto transfer = callContext.getParameterSize() > 1 ? callContext.getParameterAsValue(1) : Value::undefined(); CHECK_CALL_CONTEXT(callContext); + auto message = JavaScriptMessage::make(data, transfer, worker->getWorkerRuntime(), nullptr); + if (!message) { + return callContext.throwError(message.moveError()); + } + worker->postMessage(message.moveValue()); } return callContext.getContext().newUndefined(); } @@ -2284,7 +2348,7 @@ JSValueRef JavaScriptRuntime::workerPostMessage(JSFunctionNativeCallContext& cal JSValueRef JavaScriptRuntime::workerTerminate(JSFunctionNativeCallContext& callContext) { auto worker = thisFromCallContext(callContext); if (worker != nullptr) { - worker->close(); + worker->terminate(); } return callContext.getContext().newUndefined(); } @@ -2486,6 +2550,23 @@ void JavaScriptRuntime::buildContext(Valdi::IJavaScriptContext& context, return; } + auto messagePortClass = JavaScriptMessagePort::makeClass(context, exceptionTracker); + if (!exceptionTracker) { + return; + } + context.setObjectProperty(globalObject.get(), "MessagePort", messagePortClass.get(), exceptionTracker); + if (!exceptionTracker) { + return; + } + auto messageChannelClass = JavaScriptMessageChannel::makeClass(context, exceptionTracker); + if (!exceptionTracker) { + return; + } + context.setObjectProperty(globalObject.get(), "MessageChannel", messageChannelClass.get(), exceptionTracker); + if (!exceptionTracker) { + return; + } + auto performanceObject = context.newObject(exceptionTracker); if (!exceptionTracker) { return; @@ -2547,7 +2628,7 @@ std::vector> JavaScriptRuntime::getAllWorkers() { auto it = _jsWorkers.begin(); while (it != _jsWorkers.end()) { auto jsWorker = it->lock(); - if (jsWorker != nullptr) { + if (jsWorker != nullptr && !jsWorker->isDisposed()) { out.emplace_back(std::move(jsWorker)); it++; } else { @@ -3851,7 +3932,7 @@ DispatchFunction JavaScriptRuntime::makeJsThreadDispatchFunction(Ref&& JavaScriptThreadTask&& jsTask) { SC_ASSERT(ownerContext != nullptr); return [this, retainedContext = RetainedContext(std::move(ownerContext)), jsTask = std::move(jsTask)]() { - if (_javaScriptContext == nullptr || !_running) { + if (_isDisposed || _javaScriptContext == nullptr || !_running) { return; } diff --git a/valdi/src/valdi/runtime/JavaScript/JavaScriptRuntime.hpp b/valdi/src/valdi/runtime/JavaScript/JavaScriptRuntime.hpp index 470f366b..65c1b33c 100644 --- a/valdi/src/valdi/runtime/JavaScript/JavaScriptRuntime.hpp +++ b/valdi/src/valdi/runtime/JavaScript/JavaScriptRuntime.hpp @@ -217,6 +217,8 @@ class JavaScriptRuntime : public JavaScriptTaskScheduler, void fullTeardown(); void partialTeardown(); + void requestFullTeardown(); + void requestExecutionTermination(); void setThreadQoS(ThreadQoSClass threadQoS); @@ -546,6 +548,7 @@ class JavaScriptRuntime : public JavaScriptTaskScheduler, // Worker methods // - new Worker(...) + // - new MessageChannel() // - worker.onmessage = ... // - worker.postMessage(...) // - worker.terminate() @@ -645,6 +648,7 @@ class JavaScriptRuntime : public JavaScriptTaskScheduler, bool shouldCrash); void teardown(bool destroyContext); + void teardownOnJsThread(bool destroyContext); Ref doCaptureCurrentStackTrace(IJavaScriptContext& jsContext); diff --git a/valdi/src/valdi/runtime/JavaScript/JavaScriptUtils.cpp b/valdi/src/valdi/runtime/JavaScript/JavaScriptUtils.cpp index b073ffed..70880b7e 100644 --- a/valdi/src/valdi/runtime/JavaScript/JavaScriptUtils.cpp +++ b/valdi/src/valdi/runtime/JavaScript/JavaScriptUtils.cpp @@ -133,7 +133,15 @@ Ref unwrapWrappedObject(IJavaScriptContext& jsContext, std::optional unwrapJSValueIfNeeded(IJavaScriptContext& jsContext, RefCountable* object, + JSValueForNativeObjectResolver* nativeObjectResolver, JSExceptionTracker& exceptionTracker) { + if (nativeObjectResolver != nullptr) { + auto resolvedValue = nativeObjectResolver->getJSValueForNativeObject(object); + if (resolvedValue) { + return resolvedValue; + } + } + auto* wrappedJsValue = dynamic_cast(object); if (wrappedJsValue == nullptr) { return std::nullopt; @@ -149,7 +157,7 @@ std::optional unwrapJSValueIfNeeded(IJavaScriptContext& jsContext, JSValueRef valdiObjectToJSValue(IJavaScriptContext& jsContext, const Ref& valdiObject, JSExceptionTracker& exceptionTracker) { - auto jsValue = unwrapJSValueIfNeeded(jsContext, valdiObject.get(), exceptionTracker); + auto jsValue = unwrapJSValueIfNeeded(jsContext, valdiObject.get(), nullptr, exceptionTracker); if (!jsValue || !exceptionTracker) { return jsContext.newUndefined(); } @@ -223,6 +231,7 @@ JSValueRef proxyObjectToJSValue(IJavaScriptContext& jsContext, JSValueRef valueToJSValue(IJavaScriptContext& jsContext, const Valdi::Value& value, + JSValueForNativeObjectResolver* nativeObjectResolver, const ReferenceInfoBuilder& referenceInfoBuilder, JSExceptionTracker& exceptionTracker) { switch (value.getType()) { @@ -248,7 +257,8 @@ JSValueRef valueToJSValue(IJavaScriptContext& jsContext, return proxyObjectToJSValue( jsContext, value.getTypedProxyObjectRef(), referenceInfoBuilder, exceptionTracker); case ValueType::ValdiObject: { - auto jsValue = unwrapJSValueIfNeeded(jsContext, value.getValdiObject().get(), exceptionTracker); + auto jsValue = + unwrapJSValueIfNeeded(jsContext, value.getValdiObject().get(), nativeObjectResolver, exceptionTracker); if (!exceptionTracker) { return jsContext.newUndefined(); } @@ -268,8 +278,11 @@ JSValueRef valueToJSValue(IJavaScriptContext& jsContext, } for (const auto& it : map) { - auto value = - valueToJSValue(jsContext, it.second, referenceInfoBuilder.withProperty(it.first), exceptionTracker); + auto value = valueToJSValue(jsContext, + it.second, + nativeObjectResolver, + referenceInfoBuilder.withProperty(it.first), + exceptionTracker); if (!exceptionTracker) { return jsContext.newUndefined(); } @@ -292,8 +305,8 @@ JSValueRef valueToJSValue(IJavaScriptContext& jsContext, size_t i = 0; for (const auto& item : array) { - auto itemResult = - valueToJSValue(jsContext, item, referenceInfoBuilder.withArrayIndex(i), exceptionTracker); + auto itemResult = valueToJSValue( + jsContext, item, nativeObjectResolver, referenceInfoBuilder.withArrayIndex(i), exceptionTracker); if (!exceptionTracker) { return jsContext.newUndefined(); } @@ -318,7 +331,7 @@ JSValueRef valueToJSValue(IJavaScriptContext& jsContext, case ValueType::Function: { auto func = value.getFunctionRef(); - auto jsValue = unwrapJSValueIfNeeded(jsContext, func.get(), exceptionTracker); + auto jsValue = unwrapJSValueIfNeeded(jsContext, func.get(), nullptr, exceptionTracker); if (!exceptionTracker) { return jsContext.newUndefined(); } @@ -336,6 +349,13 @@ JSValueRef valueToJSValue(IJavaScriptContext& jsContext, SC_ASSERT_FAIL("This should not happen"); } +JSValueRef valueToJSValue(IJavaScriptContext& jsContext, + const Valdi::Value& value, + const ReferenceInfoBuilder& referenceInfoBuilder, + JSExceptionTracker& exceptionTracker) { + return valueToJSValue(jsContext, value, nullptr, referenceInfoBuilder, exceptionTracker); +} + StringBox nameFromJSFunction(IJavaScriptContext& jsContext, const JSValue& jsValue) { static auto kNameKey = STRING_LITERAL("name"); static auto kAnonymousFunction = STRING_LITERAL(""); @@ -577,7 +597,8 @@ JSValueRef newTypedArrayFromBytesView(IJavaScriptContext& jsContext, const BytesView& bytesView, JSExceptionTracker& exceptionTracker) { JSValueRef arrayBuffer; - auto unwrappedArrayBuffer = unwrapJSValueIfNeeded(jsContext, bytesView.getSource().get(), exceptionTracker); + auto unwrappedArrayBuffer = + unwrapJSValueIfNeeded(jsContext, bytesView.getSource().get(), nullptr, exceptionTracker); if (unwrappedArrayBuffer) { arrayBuffer = JSValueRef::makeRetained(jsContext, unwrappedArrayBuffer.value()); diff --git a/valdi/src/valdi/runtime/JavaScript/JavaScriptUtils.hpp b/valdi/src/valdi/runtime/JavaScript/JavaScriptUtils.hpp index d8a212e3..66ebc90c 100644 --- a/valdi/src/valdi/runtime/JavaScript/JavaScriptUtils.hpp +++ b/valdi/src/valdi/runtime/JavaScript/JavaScriptUtils.hpp @@ -18,6 +18,13 @@ namespace Valdi { class ValueFunctionWithJSValue; +class JSValueForNativeObjectResolver { +public: + virtual ~JSValueForNativeObjectResolver() = default; + + virtual std::optional getJSValueForNativeObject(RefCountable* object) = 0; +}; + Error convertJSErrorToValdiError(IJavaScriptContext& jsContext, JSValueRef jsValue, const Error* cause); JSValueRef convertValdiErrorToJSError(IJavaScriptContext& jsContext, @@ -41,6 +48,12 @@ std::string formatJsModule(const std::string_view& jsModule); std::optional getPreCompiledJsModuleData(const BytesView& jsModule); BytesView makePreCompiledJsModule(const Byte* byteCode, size_t length); +[[nodiscard]] JSValueRef valueToJSValue(IJavaScriptContext& jsContext, + const Valdi::Value& value, + JSValueForNativeObjectResolver* nativeObjectResolver, + const ReferenceInfoBuilder& referenceInfoBuilder, + JSExceptionTracker& exceptionTracker); + [[nodiscard]] JSValueRef valueToJSValue(IJavaScriptContext& jsContext, const Valdi::Value& value, const ReferenceInfoBuilder& referenceInfoBuilder, diff --git a/valdi/src/valdi/runtime/JavaScript/JavaScriptWorker.cpp b/valdi/src/valdi/runtime/JavaScript/JavaScriptWorker.cpp index 2dcd2387..1dc73b81 100644 --- a/valdi/src/valdi/runtime/JavaScript/JavaScriptWorker.cpp +++ b/valdi/src/valdi/runtime/JavaScript/JavaScriptWorker.cpp @@ -6,69 +6,77 @@ namespace Valdi { VALDI_CLASS_IMPL(JavaScriptWorker); -JavaScriptWorker::JavaScriptWorker(Ref runtime, const StringBox& url) - : _runtime(std::move(runtime)), _url(url) { - VALDI_INFO(_runtime->getLogger(), "Created JS Worker with URL: {}", url); +JavaScriptWorker::JavaScriptWorker(Ref hostRuntime, + Ref workerRuntime, + const StringBox& url) + : _hostRuntime(hostRuntime), _workerRuntime(std::move(workerRuntime)), _url(url) { + VALDI_INFO(_workerRuntime->getLogger(), "Created JS Worker with URL: {}", url); } JavaScriptWorker::~JavaScriptWorker() { - VALDI_INFO(_runtime->getLogger(), "Destroying JS Worker with URL: {}", _url); - _runtime->fullTeardown(); + VALDI_INFO(_workerRuntime->getLogger(), "Destroying JS Worker with URL: {}", _url); + _workerRuntime->requestExecutionTermination(); + _workerRuntime->requestFullTeardown(); } -// Retrieve the onmessage function set by the script -static Ref getGlobalOnMessage(JavaScriptEntryParameters& entry) { +Ref JavaScriptWorker::getWorkerRuntime() const { + return _workerRuntime; +} + +static JSValueRef getGlobalOnMessage(JavaScriptEntryParameters& entry) { auto globalObj = entry.jsContext.getGlobalObject(entry.exceptionTracker); auto onMessageKey = STRING_LITERAL("onmessage"); auto onmessage = entry.jsContext.getObjectProperty(globalObj.get(), onMessageKey.toStringView(), entry.exceptionTracker); - if (entry.jsContext.isValueFunction(onmessage.get())) { - return jsValueToFunction( - entry.jsContext, onmessage.get(), ReferenceInfoBuilder().withObject(onMessageKey), entry.exceptionTracker); - } else { - return nullptr; - } -} - -// Call the onmessage function with data -static void dispatchMessage(const Ref& func, const Value& data) { - if (func == nullptr) { - return; - } - - auto e = makeShared(); - (*e)[STRING_LITERAL("data")] = data; - (*func)({Value(e)}); + return entry.jsContext.isValueFunction(onmessage.get()) ? std::move(onmessage) : entry.jsContext.newUndefined(); } void JavaScriptWorker::postInit() { - _runtime->dispatchOnJsThread( + _workerRuntime->dispatchOnJsThread( nullptr, JavaScriptTaskScheduleTypeDefault, 0, [self = strongSmallRef(this)](JavaScriptEntryParameters& entry) { - self->doPostInit(); + if (self->isRunning()) { + self->doPostInit(); + } }); } -void JavaScriptWorker::setHostOnMessage(const Ref& func) { - _runtime->dispatchOnJsThread( - nullptr, - JavaScriptTaskScheduleTypeDefault, - 0, - [self = strongSmallRef(this), func](JavaScriptEntryParameters& entry) { self->doSetHostOnMessage(func); }); +void JavaScriptWorker::setHostOnMessage(Shared func) { + std::lock_guard lock(_mutex); + if (_state == State::Running) { + _hostOnMessage = std::move(func); + } } -void JavaScriptWorker::postMessage(const Value& value) { - _runtime->dispatchOnJsThread( - nullptr, - JavaScriptTaskScheduleTypeDefault, - 0, - [self = strongSmallRef(this), value](JavaScriptEntryParameters& entry) { self->doPostMessage(entry, value); }); +Shared JavaScriptWorker::getHostOnMessage() const { + std::lock_guard lock(_mutex); + return _hostOnMessage; +} + +void JavaScriptWorker::postMessage(const Ref& message) { + _workerRuntime->dispatchOnJsThread(nullptr, + JavaScriptTaskScheduleTypeDefault, + 0, + [self = strongSmallRef(this), message](JavaScriptEntryParameters& entry) { + self->doPostMessage(entry, message); + }); } void JavaScriptWorker::close() { - _runtime->dispatchOnJsThread(nullptr, - JavaScriptTaskScheduleTypeDefault, - 0, - [self = strongSmallRef(this)](JavaScriptEntryParameters& entry) { self->doClose(); }); + doClose(); +} + +void JavaScriptWorker::terminate() { + { + std::lock_guard lock(_mutex); + if (_state != State::Running) { + return; + } + _state = State::Terminated; + _hostOnMessage = nullptr; + } + + _workerRuntime->requestExecutionTermination(); + _workerRuntime->requestFullTeardown(); } void JavaScriptWorker::doPostInit() { @@ -78,16 +86,29 @@ void JavaScriptWorker::doPostInit() { // - postMessage // - close // - location, https://developer.mozilla.org/en-US/docs/Web/API/WorkerLocation, only href and search are populated - _runtime->setValueToGlobalObject(STRING_LITERAL("onmessage"), Value::undefined()); + _workerRuntime->setValueToGlobalObject(STRING_LITERAL("onmessage"), Value::undefined()); auto postMessageFunc = [weakSelf](const ValueFunctionCallContext& callContext) -> Value { auto self = weakSelf.lock(); - if (self && !self->_closed) { - dispatchMessage(self->_hostOnMessage, callContext.getParameter(0)); + if (self && self->isRunning()) { + auto hostRuntime = Ref(self->_hostRuntime.lock()); + if (hostRuntime == nullptr) { + return Value::undefined(); + } + auto transfer = callContext.getParametersSize() > 1 ? callContext.getParameter(1) : Value::undefinedRef(); + auto message = JavaScriptMessage::make(callContext.getParameter(0), transfer, hostRuntime, nullptr); + if (!message) { + callContext.getExceptionTracker().onError(message.moveError()); + return Value::undefined(); + } + message.value()->dispatchHandler(self->getHostOnMessage(), [weakSelf]() { + auto self = weakSelf.lock(); + return self != nullptr && self->canDeliverPendingMessage(); + }); } return Value::undefined(); }; - _runtime->setValueToGlobalObject(STRING_LITERAL("postMessage"), - Value(makeShared(postMessageFunc))); + _workerRuntime->setValueToGlobalObject(STRING_LITERAL("postMessage"), + Value(makeShared(postMessageFunc))); auto closeFunc = [weakSelf](const ValueFunctionCallContext& callContext) -> Value { auto self = weakSelf.lock(); if (self) { @@ -96,7 +117,8 @@ void JavaScriptWorker::doPostInit() { return Value::undefined(); }; - _runtime->setValueToGlobalObject(STRING_LITERAL("close"), Value(makeShared(closeFunc))); + _workerRuntime->setValueToGlobalObject(STRING_LITERAL("close"), + Value(makeShared(closeFunc))); auto queryStartIndex = _url.indexOf('?'); auto scriptUrl = queryStartIndex.has_value() ? _url.substring(0, queryStartIndex.value()) : _url; @@ -104,25 +126,41 @@ void JavaScriptWorker::doPostInit() { (*location)[STRING_LITERAL("href")] = Value(_url); (*location)[STRING_LITERAL("search")] = Value(queryStartIndex.has_value() ? _url.substring(queryStartIndex.value()) : STRING_LITERAL("")); - _runtime->setValueToGlobalObject(STRING_LITERAL("location"), Value(location)); + _workerRuntime->setValueToGlobalObject(STRING_LITERAL("location"), Value(location)); // Evaluate the worker script - auto result = _runtime->evalModuleSync(scriptUrl, false); + auto result = _workerRuntime->evalModuleSync(scriptUrl, false); } -void JavaScriptWorker::doSetHostOnMessage(const Ref& func) { - _hostOnMessage = func; +void JavaScriptWorker::doPostMessage(JavaScriptEntryParameters& entry, const Ref& message) const { + if (isRunning()) { + auto onMessage = getGlobalOnMessage(entry); + if (entry.jsContext.isValueFunction(onMessage.get())) { + message->callHandler(entry, onMessage.get()); + } + } } -void JavaScriptWorker::doPostMessage(JavaScriptEntryParameters& entry, const Value& value) const { - if (!_closed) { - auto onMessageFunc = getGlobalOnMessage(entry); - dispatchMessage(onMessageFunc, value); +void JavaScriptWorker::doClose() { + { + std::lock_guard lock(_mutex); + if (_state != State::Running) { + return; + } + _state = State::Closed; + _hostOnMessage = nullptr; } + _workerRuntime->requestFullTeardown(); } -void JavaScriptWorker::doClose() { - _closed = true; +bool JavaScriptWorker::isRunning() const { + std::lock_guard lock(_mutex); + return _state == State::Running; +} + +bool JavaScriptWorker::canDeliverPendingMessage() const { + std::lock_guard lock(_mutex); + return _state != State::Terminated; } } // namespace Valdi diff --git a/valdi/src/valdi/runtime/JavaScript/JavaScriptWorker.hpp b/valdi/src/valdi/runtime/JavaScript/JavaScriptWorker.hpp index 2e05d05a..b3fa892e 100644 --- a/valdi/src/valdi/runtime/JavaScript/JavaScriptWorker.hpp +++ b/valdi/src/valdi/runtime/JavaScript/JavaScriptWorker.hpp @@ -1,6 +1,8 @@ #pragma once +#include "valdi/runtime/JavaScript/JavaScriptMessagePort.hpp" #include "valdi/runtime/JavaScript/JavaScriptRuntime.hpp" +#include "valdi_core/cpp/Utils/Mutex.hpp" namespace Valdi { @@ -8,26 +10,38 @@ class JavaScriptWorker : public ValdiObject { public: VALDI_CLASS_HEADER(JavaScriptWorker); - JavaScriptWorker(Ref runtime, const StringBox& url); + JavaScriptWorker(Ref hostRuntime, Ref workerRuntime, const StringBox& url); ~JavaScriptWorker() override; + Ref getWorkerRuntime() const; void postInit(); - void setHostOnMessage(const Ref& func); - void postMessage(const Value& value); + void setHostOnMessage(Shared func); + void postMessage(const Ref& message); void close(); + void terminate(); private: - Ref _runtime; + enum class State { + Running, + Closed, + Terminated, + }; + + Weak _hostRuntime; + Ref _workerRuntime; const StringBox _url; - // onmessage callback in the owner's js context - Ref _hostOnMessage; - bool _closed = false; + mutable Mutex _mutex; + Shared _hostOnMessage; + State _state = State::Running; // Called from JS runtime thread void doPostInit(); - void doSetHostOnMessage(const Ref& func); - void doPostMessage(JavaScriptEntryParameters& entry, const Value& value) const; + void doPostMessage(JavaScriptEntryParameters& entry, const Ref& message) const; void doClose(); + + bool isRunning() const; + bool canDeliverPendingMessage() const; + Shared getHostOnMessage() const; }; } // namespace Valdi diff --git a/valdi/test/integration/JSIntegrationTests.cpp b/valdi/test/integration/JSIntegrationTests.cpp index 6a350883..f731fda0 100644 --- a/valdi/test/integration/JSIntegrationTests.cpp +++ b/valdi/test/integration/JSIntegrationTests.cpp @@ -5,8 +5,10 @@ #include "valdi/runtime/JavaScript/JSFunctionWithCallable.hpp" #include "valdi_core/cpp/Utils/ByteBuffer.hpp" #include "valdi_core/cpp/Utils/StaticString.hpp" +#include #include #include +#include using namespace Valdi; using namespace snap::valdi_core; @@ -962,6 +964,116 @@ TEST_P(JSContextFixture, nativeClassCallbacksDeliverInterrupts) { context.setListener(nullptr); } +TEST_P(JSContextFixture, executionTerminationIsStickyAndDoesNotNotifyTheDiagnosticListener) { + MAIN_THREAD_INIT(); + auto wrapper = createWrapper(); + auto jsEntry = wrapper.makeJsEntry(); + auto& context = jsEntry.context; + MockJavaScriptContextListener listener; + context.setListener(&listener); + + ASSERT_FALSE(context.interruptRequested()); + ASSERT_FALSE(context.executionTerminationRequested()); + + context.requestInterrupt(); + ASSERT_TRUE(context.interruptRequested()); + ASSERT_FALSE(context.onInterrupt()); + ASSERT_EQ(1, listener.interruptCount); + ASSERT_FALSE(context.interruptRequested()); + + context.requestExecutionTermination(); + ASSERT_TRUE(context.interruptRequested()); + ASSERT_TRUE(context.executionTerminationRequested()); + ASSERT_TRUE(context.onInterrupt()); + ASSERT_EQ(1, listener.interruptCount); + + ASSERT_TRUE(context.interruptRequested()); + ASSERT_TRUE(context.onInterrupt()); + ASSERT_EQ(1, listener.interruptCount); + + context.setListener(nullptr); +} + +TEST_P(JSContextFixture, executionTerminationInterruptsRunningJavaScript) { + if (isJSCore()) { + GTEST_SKIP() << "JavaScriptCore has no public API for interrupting pure JavaScript execution"; + } + + MAIN_THREAD_INIT(); + auto wrapper = createWrapper(); + auto jsEntry = wrapper.makeJsEntry(); + auto& context = jsEntry.context; + auto& exceptionTracker = jsEntry.exceptionTracker; + + std::thread terminator([&context]() { + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + context.requestExecutionTermination(); + }); + + context.evaluate("while (true) {}", "execution-termination.js", exceptionTracker); + terminator.join(); + + ASSERT_FALSE(exceptionTracker); + exceptionTracker.clearError(); +} + +TEST_P(JSContextFixture, hermesExecutionTerminationInterruptsPrecompiledJavaScript) { + if (!isHermes()) { + GTEST_SKIP() << "This verifies Hermes serialized bytecode async-break checks"; + } + + MAIN_THREAD_INIT(); + auto wrapper = createWrapper(); + auto jsEntry = wrapper.makeJsEntry(); + auto& context = jsEntry.context; + auto& exceptionTracker = jsEntry.exceptionTracker; + auto moduleData = context.preCompile("while (true) {}", "precompiled-execution-termination.js", exceptionTracker); + jsEntry.checkException(); + auto bytecode = getPreCompiledJsModuleData(moduleData); + ASSERT_TRUE(bytecode); + + auto precompiledFunction = + context.evaluatePreCompiled(bytecode.value(), "precompiled-execution-termination.js", exceptionTracker); + jsEntry.checkException(); + + std::thread terminator([&context]() { + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + context.requestExecutionTermination(); + }); + + JSFunctionCallContext callContext(context, nullptr, 0, exceptionTracker); + context.callObjectAsFunction(precompiledFunction.get(), callContext); + terminator.join(); + + ASSERT_FALSE(exceptionTracker); + exceptionTracker.clearError(); +} + +TEST_P(JSContextFixture, javaScriptCoreExecutionTerminationInterruptsAtNativeCallBoundary) { + if (!isJSCore()) { + GTEST_SKIP() << "This verifies JavaScriptCore's best-effort native-call checkpoint"; + } + + MAIN_THREAD_INIT(); + auto wrapper = createWrapper(); + auto jsEntry = wrapper.makeJsEntry(); + auto& context = jsEntry.context; + auto& exceptionTracker = jsEntry.exceptionTracker; + setUpNativeCounterTest(jsEntry, &constructNativeCounter); + + std::thread terminator([&context]() { + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + context.requestExecutionTermination(); + }); + + context.evaluate( + "while (true) { NativeCounter.twice(1); }", "native-boundary-execution-termination.js", exceptionTracker); + terminator.join(); + + ASSERT_FALSE(exceptionTracker); + exceptionTracker.clearError(); +} + TEST_P(JSContextFixture, nativeClassRejectsInvalidUsage) { MAIN_THREAD_INIT(); auto wrapper = createWrapper(); diff --git a/valdi/test/integration/Runtime_tests.cpp b/valdi/test/integration/Runtime_tests.cpp index 6fbb3373..a43d7952 100644 --- a/valdi/test/integration/Runtime_tests.cpp +++ b/valdi/test/integration/Runtime_tests.cpp @@ -6174,6 +6174,54 @@ TEST_P(RuntimeFixture, FLAKY_workerWorks) { ASSERT_EQ(res.toString(), "works"); } +static Value callWorkerTestMethod(RuntimeWrapper& wrapper, + const Ref& context, + const StringBox& methodName) { + auto valuePromise = std::make_shared>(); + auto completed = std::make_shared(false); + auto valueFuture = valuePromise->get_future(); + auto callback = makeShared([valuePromise, completed](const auto& callContext) { + if (!completed->exchange(true)) { + valuePromise->set_value(callContext.getParameter(0)); + } + return Value::undefined(); + }); + wrapper.runtime->getJavaScriptRuntime()->callComponentFunction( + context, methodName, ValueArray::make({Value(callback)})); + + auto status = valueFuture.wait_for(std::chrono::seconds(5)); + EXPECT_EQ(std::future_status::ready, status); + if (status != std::future_status::ready) { + return Value::undefined(); + } + return valueFuture.get(); +} + +TEST_P(RuntimeFixture, workerCanBeTerminatedBeforeInitialization) { + auto tree = + wrapper.createViewNodeTreeAndContext(STRING_LITERAL("WorkerTest@test/src/WorkerTest"), Value(), Value()); + wrapper.waitUntilAllUpdatesCompleted(); + + auto result = + callWorkerTestMethod(wrapper, tree->getContext(), STRING_LITERAL("terminateBeforeInitialization")); + ASSERT_TRUE(result.isString()); + EXPECT_EQ("works", result.toString()); +} + +TEST_P(RuntimeFixture, busyWorkerCanBeTerminated) { + if (isJSCore()) { + GTEST_SKIP() << "JavaScriptCore cannot interrupt a pure JavaScript loop through its public API"; + } + + auto tree = + wrapper.createViewNodeTreeAndContext(STRING_LITERAL("WorkerTest@test/src/WorkerTest"), Value(), Value()); + wrapper.waitUntilAllUpdatesCompleted(); + + auto result = callWorkerTestMethod(wrapper, tree->getContext(), STRING_LITERAL("terminateBusyWorker")); + ASSERT_TRUE(result.isString()); + EXPECT_EQ("works", result.toString()); +} + TEST_P(RuntimeFixture, canLockAllJSContexts) { auto tree1 = wrapper.createViewNodeTreeAndContext(STRING_LITERAL("WorkerTest@test/src/WorkerTest"), Value(), Value()); diff --git a/valdi/testdata/resources/modules/test/src/InfiniteWorker.ts b/valdi/testdata/resources/modules/test/src/InfiniteWorker.ts new file mode 100644 index 00000000..3cf79946 --- /dev/null +++ b/valdi/testdata/resources/modules/test/src/InfiniteWorker.ts @@ -0,0 +1,4 @@ +postMessage('ready'); +postMessage('late'); + +while (true) {} diff --git a/valdi/testdata/resources/modules/test/src/WorkerTest.tsx b/valdi/testdata/resources/modules/test/src/WorkerTest.tsx index a4fa1433..669e48bd 100644 --- a/valdi/testdata/resources/modules/test/src/WorkerTest.tsx +++ b/valdi/testdata/resources/modules/test/src/WorkerTest.tsx @@ -12,7 +12,44 @@ export class WorkerTest extends Component { this.worker.postMessage('hi'); } + terminateBusyWorker(callback: (res: string) => void) { + const busyWorker = new Worker('test/src/InfiniteWorker'); + this.worker = busyWorker; + busyWorker.onmessage = e => { + if (e.data !== 'ready') { + callback('message-after-termination'); + return; + } + + busyWorker.terminate(); + // Termination must remain idempotent while asynchronous teardown is pending. + busyWorker.terminate(); + + const replacementWorker = new Worker('test/src/MyWorker'); + this.worker = replacementWorker; + replacementWorker.onmessage = replacementEvent => { + callback(replacementEvent.data as string); + }; + replacementWorker.postMessage('hi'); + }; + } + + terminateBeforeInitialization(callback: (res: string) => void) { + const terminatedWorker = new Worker('test/src/MyWorker'); + terminatedWorker.terminate(); + // Termination must also be idempotent before worker initialization completes. + terminatedWorker.terminate(); + terminatedWorker.postMessage('ignored'); + + const replacementWorker = new Worker('test/src/MyWorker'); + this.worker = replacementWorker; + replacementWorker.onmessage = e => { + callback(e.data as string); + }; + replacementWorker.postMessage('hi'); + } + onRender() { - + ; } } diff --git a/valdi_core/src/valdi_core/cpp/Threading/GCDDispatchQueue.cpp b/valdi_core/src/valdi_core/cpp/Threading/GCDDispatchQueue.cpp index 9feaa34a..eb36dd65 100644 --- a/valdi_core/src/valdi_core/cpp/Threading/GCDDispatchQueue.cpp +++ b/valdi_core/src/valdi_core/cpp/Threading/GCDDispatchQueue.cpp @@ -245,6 +245,10 @@ void GCDDispatchQueue::decrementPendingTaskCount() { void GCDDispatchQueue::fullTeardown() { teardown(); + if (isCurrent()) { + return; + } + // Try to acquire the execution lock with timeout to ensure no tasks are executing // If we can acquire it, all tasks have completed. If timeout, proceed anyway to // avoid indefinite hang (preserves COMPOSER-837 JSCore deadlock workaround). From d2c3b7418f5eb4c719bd48dd8f3c952a07fe129b Mon Sep 17 00:00:00 2001 From: Simon Corsin Date: Thu, 23 Jul 2026 15:03:56 -0500 Subject: [PATCH 2/3] Fix worker teardown lifetime and initialization termination --- .../valdi/jscore/JavaScriptCoreContext.hpp | 2 - .../valdi/runtime/Context/ContextManager.hpp | 4 +- .../runtime/JavaScript/JavaScriptRuntime.cpp | 40 ++++++++++--------- .../runtime/JavaScript/JavaScriptRuntime.hpp | 4 +- valdi/src/valdi/runtime/Runtime.cpp | 40 +++++++++---------- valdi/src/valdi/runtime/Runtime.hpp | 2 +- 6 files changed, 47 insertions(+), 45 deletions(-) diff --git a/valdi/src/valdi/jscore/JavaScriptCoreContext.hpp b/valdi/src/valdi/jscore/JavaScriptCoreContext.hpp index dc2e5902..17c3a7c2 100644 --- a/valdi/src/valdi/jscore/JavaScriptCoreContext.hpp +++ b/valdi/src/valdi/jscore/JavaScriptCoreContext.hpp @@ -179,8 +179,6 @@ class JavaScriptCoreContext : public Valdi::IJavaScriptContext { const Valdi::JSPropertyName& getPrototypePropertyName() const; - const Valdi::JSPropertyName& getPrototypePropertyName() const; - protected: Valdi::JSValueRef onNewBool(bool boolean) override; diff --git a/valdi/src/valdi/runtime/Context/ContextManager.hpp b/valdi/src/valdi/runtime/Context/ContextManager.hpp index 79c6954c..9d9af800 100644 --- a/valdi/src/valdi/runtime/Context/ContextManager.hpp +++ b/valdi/src/valdi/runtime/Context/ContextManager.hpp @@ -40,10 +40,10 @@ class IContextManagerListener { * Can create a Context from a loaded document and associate them with an id. It holds the shared pointers for all * contexts. The shared pointers get released when DestroyContext() is called. */ -class ContextManager { +class ContextManager : public SimpleRefCountable { public: ContextManager(const Ref& logger, Runtime* runtime); - ~ContextManager(); + ~ContextManager() override; SharedContext createContext(Ref handler, const Ref& viewManagerContext, diff --git a/valdi/src/valdi/runtime/JavaScript/JavaScriptRuntime.cpp b/valdi/src/valdi/runtime/JavaScript/JavaScriptRuntime.cpp index c99686a2..f8bd2c29 100644 --- a/valdi/src/valdi/runtime/JavaScript/JavaScriptRuntime.cpp +++ b/valdi/src/valdi/runtime/JavaScript/JavaScriptRuntime.cpp @@ -301,7 +301,7 @@ std::optional JavaScriptStacktraceCaptureSession:: JavaScriptRuntime::JavaScriptRuntime(IJavaScriptBridge& jsBridge, ResourceManager& resourceManager, - ContextManager& contextManager, + const Ref& contextManager, MainThreadManager& mainThreadManager, AttributeIds& attributeIds, PlatformType platformType, @@ -346,7 +346,7 @@ JavaScriptRuntime::JavaScriptRuntime(IJavaScriptBridge& jsBridge, this->doInitialize(); }); - _globalContext = _contextManager.createContext(nullptr, nullptr, /* deferRender */ true); + _globalContext = _contextManager->createContext(nullptr, nullptr, /* deferRender */ true); // Keep a +1 disposable until we complete the teardown _globalContext->retainDisposables(); } @@ -485,7 +485,7 @@ void JavaScriptRuntime::teardownOnJsThread(bool destroyContext) { auto nonDeferredPool = Valdi::RefCountableAutoreleasePool::makeNonDeferred(); _globalContext->releaseDisposables(); - _contextManager.destroyContext(_globalContext); + _contextManager->destroyContext(_globalContext); _modules.clear(); _daemonClients.clear(); @@ -844,7 +844,7 @@ JSValueRef JavaScriptRuntime::runtimeCreateContext(JSFunctionNativeCallContext& Ref context; if (callContext.getContext().isValueUndefined(jsContextHandler)) { - context = _contextManager.createContext(nullptr, nullptr, /* deferRender */ true); + context = _contextManager->createContext(nullptr, nullptr, /* deferRender */ true); } else { if (_defaultViewManagerContext == nullptr) { return callContext.throwError(Error("Cannot create context without a default ViewManagerContext")); @@ -859,7 +859,7 @@ JSValueRef JavaScriptRuntime::runtimeCreateContext(JSFunctionNativeCallContext& true); handler->setJsContextHandler(globalRef); - context = _contextManager.createContext(handler, _defaultViewManagerContext, /* deferRender */ true); + context = _contextManager->createContext(handler, _defaultViewManagerContext, /* deferRender */ true); } context->onCreate(); @@ -887,12 +887,12 @@ JSValueRef JavaScriptRuntime::runtimeDestroyContext(JSFunctionNativeCallContext& auto contextId = getParameterAsContextId(callContext, 0); CHECK_CALL_CONTEXT(callContext); - auto context = _contextManager.getContext(contextId); + auto context = _contextManager->getContext(contextId); if (context == nullptr) { return callContext.getContext().newUndefined(); } - _contextManager.destroyContext(context); + _contextManager->destroyContext(context); return callContext.getContext().newUndefined(); } @@ -900,7 +900,7 @@ JSValueRef JavaScriptRuntime::runtimeSetLayoutSpecs(JSFunctionNativeCallContext& auto contextId = getParameterAsContextId(callContext, 0); CHECK_CALL_CONTEXT(callContext); - auto context = _contextManager.getContext(contextId); + auto context = _contextManager->getContext(contextId); if (context == nullptr) { return callContext.getContext().newUndefined(); } @@ -947,7 +947,7 @@ JSValueRef JavaScriptRuntime::runtimeMeasureContext(JSFunctionNativeCallContext& auto contextId = getParameterAsContextId(callContext, 0); CHECK_CALL_CONTEXT(callContext); - auto context = _contextManager.getContext(contextId); + auto context = _contextManager->getContext(contextId); if (context == nullptr) { return callContext.getContext().newUndefined(); } @@ -1024,7 +1024,7 @@ JSValueRef JavaScriptRuntime::runtimeSubmitRenderRequest(JSFunctionNativeCallCon CHECK_CALL_CONTEXT(callContext); auto treeId = static_cast(jsContext.valueToInt(treeIdValue.get(), exceptionTracker)); CHECK_CALL_CONTEXT(callContext); - auto treeContext = _contextManager.getContext(treeId); + auto treeContext = _contextManager->getContext(treeId); if (treeContext != nullptr && !treeContext->isDestroyed()) { VALDI_INFO(*_logger, "Render request context fix: overriding destroyed context (current={}, root={}) with treeCtx={}", @@ -1196,7 +1196,7 @@ JSValueRef JavaScriptRuntime::runtimeGetNativeNodeForElementId(JSFunctionNativeC auto nodeId = static_cast(callContext.getParameterAsInt(1)); CHECK_CALL_CONTEXT(callContext); - auto context = _contextManager.getContext(contextId); + auto context = _contextManager->getContext(contextId); if (context == nullptr) { return callContext.getContext().newUndefined(); } @@ -1282,7 +1282,7 @@ JSValueRef JavaScriptRuntime::handleViewNodeSpecificAction( ReferenceInfoBuilder(callContext.getReferenceInfo()).withParameter(callbackParameterIndex), callContext.getExceptionTracker()); - auto context = _contextManager.getContext(contextId); + auto context = _contextManager->getContext(contextId); if (context == nullptr) { return callContext.throwError(Valdi::Error(STRING_FORMAT("Could not resolve Context {}", contextId))); } @@ -2670,7 +2670,7 @@ void JavaScriptRuntime::ensureValdiModuleIsLoaded(JavaScriptEntryParameters& jsE bool JavaScriptRuntime::callComponentFunction(ContextId contextId, const StringBox& functionName, const Ref& additionalParameters) { - auto context = _contextManager.getContext(contextId); + auto context = _contextManager->getContext(contextId); if (context == nullptr) { return false; } @@ -2936,7 +2936,7 @@ void JavaScriptRuntime::unloadUnusedModules(DispatchFunction completion) { FlatSet JavaScriptRuntime::getAllUsedModules() const { FlatSet allUsedModules; - for (const auto& context : _contextManager.getAllContexts()) { + for (const auto& context : _contextManager->getAllContexts()) { allUsedModules.insert(context->getPath().getResourceId()); } for (const auto& it : _modules) { @@ -3496,9 +3496,9 @@ void JavaScriptRuntime::warmUpValueMarshaller(const Value& value) { std::shared_ptr JavaScriptRuntime::createNativeObjectsManager( const std::string& scopeName) { auto scopeNameBox = scopeName.empty() ? StringBox() : StringCache::getGlobal().makeString(scopeName); - auto context = _contextManager.createContext(nullptr, nullptr, /* deferRender */ true, scopeNameBox); + auto context = _contextManager->createContext(nullptr, nullptr, /* deferRender */ true, scopeNameBox); - return makeShared(_contextManager, std::move(context)); + return makeShared(*_contextManager, std::move(context)); } void JavaScriptRuntime::destroyNativeObjectsManager( @@ -3912,7 +3912,7 @@ bool JavaScriptRuntime::isInJsThread() { } Ref JavaScriptRuntime::getLastDispatchedContext() const { - return _contextManager.getContext(_lastDispatchedContextId.load()); + return _contextManager->getContext(_lastDispatchedContextId.load()); } void JavaScriptRuntime::setModuleLoadDiagnosticsEnabled(bool enabled) { @@ -3964,6 +3964,10 @@ DispatchFunction JavaScriptRuntime::makeJsThreadDispatchFunction(Ref&& void JavaScriptRuntime::onInitError(std::string_view failingAction, const Error& error) { _running = false; + if (_isDisposed || (_javaScriptContext != nullptr && _javaScriptContext->executionTerminationRequested())) { + return; + } + handleUncaughtJsErrorNoHandler( nullptr, error.rethrow(STRING_FORMAT("Fatal init error with performing action '{}'", failingAction)), @@ -4126,7 +4130,7 @@ Result JavaScriptRuntime::dumpHeap() { Result> JavaScriptRuntime::getContextForId(ContextId contextId) const { // We lookup in the ContextManager first, to handle both contexts that are created externally // and contexts that are created directly in JS (which happens when running tests) - Ref context = _contextManager.getContext(contextId); + Ref context = _contextManager->getContext(contextId); if (context == nullptr) { // Fallback on a lookup on our contextHandler, in case the context was destroyed outside of the js thread return _contextHandler->getContextForId(contextId); diff --git a/valdi/src/valdi/runtime/JavaScript/JavaScriptRuntime.hpp b/valdi/src/valdi/runtime/JavaScript/JavaScriptRuntime.hpp index 65c1b33c..ef1d59fb 100644 --- a/valdi/src/valdi/runtime/JavaScript/JavaScriptRuntime.hpp +++ b/valdi/src/valdi/runtime/JavaScript/JavaScriptRuntime.hpp @@ -171,7 +171,7 @@ class JavaScriptRuntime : public JavaScriptTaskScheduler, public: JavaScriptRuntime(IJavaScriptBridge& jsBridge, ResourceManager& resourceManager, - ContextManager& contextManager, + const Ref& contextManager, MainThreadManager& mainThreadManager, AttributeIds& attributeIds, PlatformType platformType, @@ -362,7 +362,7 @@ class JavaScriptRuntime : public JavaScriptTaskScheduler, IJavaScriptBridge& _javaScriptBridge; Ref _javaScriptContext; ResourceManager& _resourceManager; - ContextManager& _contextManager; + Ref _contextManager; MainThreadManager& _mainThreadManager; AttributeIds& _attributeIds; [[maybe_unused]] Ref _logger; diff --git a/valdi/src/valdi/runtime/Runtime.cpp b/valdi/src/valdi/runtime/Runtime.cpp index 51d9fa94..860c61d0 100644 --- a/valdi/src/valdi/runtime/Runtime.cpp +++ b/valdi/src/valdi/runtime/Runtime.cpp @@ -144,7 +144,7 @@ Runtime::Runtime(AttributeIds& attributeIds, deviceDensity, debuggerServiceEnabled, *logger)), - _contextManager(logger, this), + _contextManager(makeShared(logger, this)), _viewNodeManager(*mainThreadManager, *logger), _mainThreadManager(mainThreadManager), _colorPalette(colorPalette), @@ -186,7 +186,7 @@ void Runtime::fullTeardown() { VALDI_INFO(*_logger, "Tearing down Valdi Runtime"); } _viewNodeManager.removeAllViewNodeTrees(); - _contextManager.destroyAllContexts(); + _contextManager->destroyAllContexts(); if (_javaScriptRuntime != nullptr) { _javaScriptRuntime->fullTeardown(); @@ -204,7 +204,7 @@ void Runtime::postInit() { _javaScriptRuntime->setModuleLoadDiagnosticsEnabled(enableModuleLoadDiagnostics()); } _viewNodeManager.setRuntime(weakRef(this)); - _contextManager.setListener(this); + _contextManager->setListener(this); if (_javaScriptRuntime != nullptr) { _javaScriptRuntime->postInit(); @@ -258,13 +258,13 @@ SharedContext Runtime::createContext(const Ref& viewManagerC contextHandler = _javaScriptRuntime->getContextHandler(); } - auto context = _contextManager.createContext(contextHandler, - viewManagerContext, - componentPath, - initialViewModel, - componentContext, - _shouldProcessUpdatesSynchronously, - deferRender); + auto context = _contextManager->createContext(contextHandler, + viewManagerContext, + componentPath, + initialViewModel, + componentContext, + _shouldProcessUpdatesSynchronously, + deferRender); _resourceManager->preloadForComponentPath(componentPath); @@ -306,7 +306,7 @@ SharedViewNodeTree Runtime::getOrCreateViewNodeTreeForContextId(ContextId contex return tree; } - auto context = _contextManager.getContext(contextId); + auto context = _contextManager->getContext(contextId); if (context == nullptr) { return nullptr; } @@ -334,7 +334,7 @@ void Runtime::doDestroyContext(const SharedContext& context) { ScopedMetrics metrics = Metrics::scopedDestroyContextLatency(getMetrics(), context->getPath().getResourceId().bundleName); - _contextManager.destroyContext(context); + _contextManager->destroyContext(context); auto viewNodeTree = _viewNodeManager.getViewNodeTreeForContextId(context->getContextId()); if (viewNodeTree != nullptr) { destroyViewNodeTree(*viewNodeTree); @@ -398,7 +398,7 @@ DumpedLogs Runtime::dumpContextsLogs() const { ValueArrayBuilder builder; - auto contexts = _contextManager.getAllContexts(); + auto contexts = _contextManager->getAllContexts(); std::sort(contexts.begin(), contexts.end(), [](const auto& left, const auto& right) -> bool { return left->getContextId() < right->getContextId(); }); @@ -604,7 +604,7 @@ void Runtime::setAutoRenderDisabled(bool autoRenderDisabled) { auto oldValue = _autoRenderDisabled.exchange(autoRenderDisabled); if (!autoRenderDisabled && oldValue) { - for (const auto& context : _contextManager.getAllContexts()) { + for (const auto& context : _contextManager->getAllContexts()) { if (context->hasPendingRenderRequests()) { getMainThreadManager().dispatch(context, [context]() { context->flushRenderRequests(); }); } @@ -613,7 +613,7 @@ void Runtime::setAutoRenderDisabled(bool autoRenderDisabled) { } void Runtime::receivedRenderRequest(const Ref& renderRequest) { - auto context = _contextManager.getContext(renderRequest->getContextId()); + auto context = _contextManager->getContext(renderRequest->getContextId()); if (context == nullptr) { return; } @@ -708,7 +708,7 @@ void Runtime::processRenderRequest(const Ref& rawRenderRequest) { void Runtime::receivedCallActionMessage(const ContextId& contextId, const StringBox& actionName, const Ref& parameters) { - auto context = _contextManager.getContext(contextId); + auto context = _contextManager->getContext(contextId); if (context == nullptr) { VALDI_WARN(*_logger, "Cannot call action: Context {} was already destroyed.", contextId); return; @@ -834,7 +834,7 @@ void Runtime::onContextCreated(const SharedContext& context) { void Runtime::onContextDestroyed(Context& context) { if (_listener != nullptr) { _listener->onContextDestroyed(*this, context); - if (_contextManager.getContextsSize() == 0) { + if (_contextManager->getContextsSize() == 0) { _listener->onAllContextsDestroyed(*this); } } @@ -861,11 +861,11 @@ MainThreadManager& Runtime::getMainThreadManager() { } const ContextManager& Runtime::getContextManager() const { - return _contextManager; + return *_contextManager; } ContextManager& Runtime::getContextManager() { - return _contextManager; + return *_contextManager; } const ViewNodeTreeManager& Runtime::getViewNodeTreeManager() const { @@ -914,7 +914,7 @@ void Runtime::setRuntimeMessageHandler(const SharedgetAllContexts()) { context->setUpdateHandlerSynchronously(shouldProcessUpdatesSynchronously); } } diff --git a/valdi/src/valdi/runtime/Runtime.hpp b/valdi/src/valdi/runtime/Runtime.hpp index 6e79df37..637bbfac 100644 --- a/valdi/src/valdi/runtime/Runtime.hpp +++ b/valdi/src/valdi/runtime/Runtime.hpp @@ -311,7 +311,7 @@ class Runtime final : public IDebuggerServiceListener, IJavaScriptRuntimeListene PlatformType _platformType; Ref _resourceManager; ResourceReloaderThrottler _reloaderThrottler; - ContextManager _contextManager; + Ref _contextManager; ViewNodeTreeManager _viewNodeManager; Ref _mainThreadManager; Ref _colorPalette; From fb3bdb65159f0ccf04f980f189a196fe493850fe Mon Sep 17 00:00:00 2001 From: Simon Corsin Date: Thu, 23 Jul 2026 15:52:34 -0500 Subject: [PATCH 3/3] Fix reentrant JavaScript promise draining --- .../valdi/hermes/HermesJavaScriptContext.cpp | 18 ++++--- .../valdi/jscore/JavaScriptCoreContext.cpp | 34 +++++++------ .../quickjs/QuickJSJavaScriptContext.cpp | 42 +++++++++------- valdi/test/integration/JSIntegrationTests.cpp | 49 +++++++++++++++++++ 4 files changed, 102 insertions(+), 41 deletions(-) diff --git a/valdi/src/valdi/hermes/HermesJavaScriptContext.cpp b/valdi/src/valdi/hermes/HermesJavaScriptContext.cpp index 03d3ee12..0b762368 100644 --- a/valdi/src/valdi/hermes/HermesJavaScriptContext.cpp +++ b/valdi/src/valdi/hermes/HermesJavaScriptContext.cpp @@ -16,6 +16,7 @@ #include "valdi/runtime/JavaScript/JavaScriptUtils.hpp" #include "valdi_core/cpp/Text/UTF16Utils.hpp" +#include "valdi_core/cpp/Utils/Defer.hpp" #include "valdi_core/cpp/Utils/ReferenceInfo.hpp" #include "valdi_core/cpp/Utils/StaticString.hpp" #include "valdi_core/cpp/Utils/StringCache.hpp" @@ -1400,15 +1401,18 @@ void HermesJavaScriptContext::requestExecutionTermination() { void HermesJavaScriptContext::willExitVM(JSExceptionTracker& exceptionTracker) { SC_ASSERT(_enterVMCount > 0); - --_enterVMCount; - if (_enterVMCount == 0) { - if (executionTerminationRequested()) { - return; - } + if (_enterVMCount > 1) { + --_enterVMCount; + return; + } - auto result = _runtime->drainJobs(); - checkException(result, exceptionTracker); + Valdi::Defer exitVM([this]() { --_enterVMCount; }); + if (executionTerminationRequested()) { + return; } + + auto result = _runtime->drainJobs(); + checkException(result, exceptionTracker); } void HermesJavaScriptContext::startDebugger(bool isWorker) { diff --git a/valdi/src/valdi/jscore/JavaScriptCoreContext.cpp b/valdi/src/valdi/jscore/JavaScriptCoreContext.cpp index c4c43f81..5755413c 100644 --- a/valdi/src/valdi/jscore/JavaScriptCoreContext.cpp +++ b/valdi/src/valdi/jscore/JavaScriptCoreContext.cpp @@ -19,6 +19,7 @@ #include "valdi_core/cpp/Utils/SmallVector.hpp" #include "valdi_core/cpp/Constants.hpp" +#include "valdi_core/cpp/Utils/Defer.hpp" #include "valdi_core/cpp/Utils/LoggerUtils.hpp" #include "valdi_core/cpp/Utils/StaticString.hpp" #include "valdi_core/cpp/Utils/ValueTypedArray.hpp" @@ -1335,25 +1336,28 @@ void JavaScriptCoreContext::requestExecutionTermination() { } void JavaScriptCoreContext::willExitVM(Valdi::JSExceptionTracker& exceptionTracker) { - auto enterVMCount = --_enterVmCount; - if (enterVMCount == 0) { - if (executionTerminationRequested()) { - return; - } + if (_enterVmCount > 1) { + --_enterVmCount; + return; + } - while (!_microtasks.empty()) { - auto microtask = _microtasks.front(); - _microtasks.pop_front(); + Valdi::Defer exitVM([this]() { --_enterVmCount; }); + if (executionTerminationRequested()) { + return; + } - JSValueRef exception = nullptr; - JSObjectCallAsFunction(getJSGlobalContext(), microtask, nullptr, 0, nullptr, &exception); + while (!_microtasks.empty()) { + auto microtask = _microtasks.front(); + _microtasks.pop_front(); - JSValueUnprotect(_globalContext, microtask); + JSValueRef exception = nullptr; + JSObjectCallAsFunction(getJSGlobalContext(), microtask, nullptr, 0, nullptr, &exception); - if (exception != nullptr) { - storeException(exceptionTracker, exception); - return; - } + JSValueUnprotect(_globalContext, microtask); + + if (exception != nullptr) { + storeException(exceptionTracker, exception); + return; } } } diff --git a/valdi/src/valdi/quickjs/QuickJSJavaScriptContext.cpp b/valdi/src/valdi/quickjs/QuickJSJavaScriptContext.cpp index 1f844ed6..3ff01e0e 100644 --- a/valdi/src/valdi/quickjs/QuickJSJavaScriptContext.cpp +++ b/valdi/src/valdi/quickjs/QuickJSJavaScriptContext.cpp @@ -15,6 +15,7 @@ #include "valdi/quickjs/QuickJSUtils.hpp" #include "valdi_core/cpp/Constants.hpp" #include "valdi_core/cpp/Utils/ByteBuffer.hpp" +#include "valdi_core/cpp/Utils/Defer.hpp" #include "valdi_core/cpp/Utils/Format.hpp" #include "valdi_core/cpp/Utils/StaticString.hpp" #include "valdi_core/cpp/Utils/StringCache.hpp" @@ -1536,26 +1537,29 @@ void QuickJSJavaScriptContext::notifyRejectedPromises() { void QuickJSJavaScriptContext::willExitVM(Valdi::JSExceptionTracker& exceptionTracker) { auto guard = _threadAccessChecker.guard(); SC_ASSERT(_enterVmCount > 0); - --_enterVmCount; - if (_enterVmCount == 0) { - if (executionTerminationRequested()) { - return; - } + if (_enterVmCount > 1) { + --_enterVmCount; + return; + } - JSContext* context = nullptr; - - for (;;) { - switch (JS_ExecutePendingJob(_runtime, &context)) { - case 0: - notifyRejectedPromises(); - return; - case 1: - continue; - case -1: { - setExceptionToTracker(exceptionTracker); - notifyRejectedPromises(); - return; - } + Valdi::Defer exitVM([this]() { --_enterVmCount; }); + if (executionTerminationRequested()) { + return; + } + + JSContext* context = nullptr; + + for (;;) { + switch (JS_ExecutePendingJob(_runtime, &context)) { + case 0: + notifyRejectedPromises(); + return; + case 1: + continue; + case -1: { + setExceptionToTracker(exceptionTracker); + notifyRejectedPromises(); + return; } } } diff --git a/valdi/test/integration/JSIntegrationTests.cpp b/valdi/test/integration/JSIntegrationTests.cpp index f731fda0..4243bf80 100644 --- a/valdi/test/integration/JSIntegrationTests.cpp +++ b/valdi/test/integration/JSIntegrationTests.cpp @@ -1971,6 +1971,55 @@ TEST_P(JSContextFixture, supportsPromise) { ASSERT_EQ(Value().setMapValue("value", Value(2.0)), result); } +TEST_P(JSContextFixture, drainsPromiseJobsWithoutReenteringPendingJobs) { + MAIN_THREAD_INIT(); + + auto wrapper = createWrapper(); + { + auto jsEntry = wrapper.makeJsEntry(); + auto& context = jsEntry.context; + auto& exceptionTracker = jsEntry.exceptionTracker; + auto globalObject = context.getGlobalObject(exceptionTracker); + jsEntry.checkException(); + + auto reenterVM = context.newFunction( + makeShared(ReferenceInfoBuilder().withObject(STRING_LITERAL("reenterVM")), + [&wrapper](auto& callContext) -> JSValueRef { + auto nestedEntry = wrapper.makeJsEntry(); + nestedEntry.checkException(); + return callContext.getContext().newUndefined(); + }), + exceptionTracker); + jsEntry.checkException(); + context.setObjectProperty(globalObject.get(), "reenterVM", reenterVM.get(), exceptionTracker); + jsEntry.checkException(); + + context.evaluate(R""""( + (() => { + globalThis.microtaskEvents = []; + Promise.resolve().then(() => { + globalThis.microtaskEvents.push('first:start'); + reenterVM(); + globalThis.microtaskEvents.push('first:end'); + }); + Promise.resolve().then(() => { + globalThis.microtaskEvents.push('second'); + }); + })() + )"""", + "nested-promise-jobs.js", + exceptionTracker); + jsEntry.checkException(); + } + + auto result = wrapper.evaluateScript("globalThis.microtaskEvents"); + auto expected = ValueArrayBuilder(); + expected.append(Value(STRING_LITERAL("first:start"))); + expected.append(Value(STRING_LITERAL("first:end"))); + expected.append(Value(STRING_LITERAL("second"))); + ASSERT_EQ(Value(expected.build()), result); +} + TEST_P(JSContextFixture, callsUnhandledPromiseCallbackWhenExceptionIsThrown) { SKIP_IF_V8("Ticket: 2259"); SKIP_IF_HERMES("Unhandled Promise Callback is not yet supported in Hermes");