From a1a13349a7aeccaaa23a37ed467d19840dda3e88 Mon Sep 17 00:00:00 2001 From: Rhazarian Date: Sun, 4 Feb 2024 00:39:49 +0100 Subject: [PATCH 1/2] fix promise --- src/lualib/Await.ts | 56 +++--- src/lualib/Promise.ts | 226 ++++++++++++++----------- test/unit/builtins/async-await.spec.ts | 25 +++ test/unit/builtins/promise.spec.ts | 20 +++ 4 files changed, 207 insertions(+), 120 deletions(-) diff --git a/src/lualib/Await.ts b/src/lualib/Await.ts index 7093a617f..ea1ade3ac 100644 --- a/src/lualib/Await.ts +++ b/src/lualib/Await.ts @@ -16,44 +16,58 @@ import { __TS__Promise } from "./Promise"; +const cocreate = coroutine.create; +const coresume = coroutine.resume; +const costatus = coroutine.status; +const coyield = coroutine.yield; + +// The precise implementation type is important here because we use special functions due to necessary optimizations +function adopt(this: void, value: unknown): __TS__Promise { + return value instanceof __TS__Promise ? value : (__TS__Promise.resolve(value) as __TS__Promise); +} + +// Be extremely careful editing this function. A single non-tail function call may ruin chained awaits performance // eslint-disable-next-line @typescript-eslint/promise-function-async export function __TS__AsyncAwaiter(this: void, generator: (this: void) => void) { return new Promise((resolve, reject) => { let resolved = false; - const asyncCoroutine = coroutine.create(generator); + const asyncCoroutine = cocreate(generator); - // eslint-disable-next-line @typescript-eslint/promise-function-async - function adopt(value: unknown) { - return value instanceof __TS__Promise ? value : Promise.resolve(value); - } - function fulfilled(value: unknown) { - const [success, resultOrError] = coroutine.resume(asyncCoroutine, value); + function fulfilled(value: unknown): void { + const [success, resultOrError] = coresume(asyncCoroutine, value); if (success) { - step(resultOrError); - } else { - reject(resultOrError); + // `step` never throws. Tail call return is important! + return step(resultOrError); } + // `reject` should never throw. Tail call return is important! + return reject(resultOrError); } - function step(result: unknown) { - if (resolved) return; - if (coroutine.status(asyncCoroutine) === "dead") { - resolve(result); - } else { - adopt(result).then(fulfilled, reject); + + function step(this: void, result: unknown): void { + if (resolved) { + return; + } + if (costatus(asyncCoroutine) === "dead") { + // `resolve` never throws. Tail call return is important! + return resolve(result); } + // We cannot use `then` because we need to avoid calling `coroutine.resume` from inside `pcall` + // `fulfilled` and `reject` should never throw. Tail call return is important! + return adopt(result).addCallbacks(fulfilled, reject); } - const [success, resultOrError] = coroutine.resume(asyncCoroutine, (v: unknown) => { + + const [success, resultOrError] = coresume(asyncCoroutine, (v: unknown) => { resolved = true; - adopt(v).then(resolve, reject); + return adopt(v).addCallbacks(resolve, reject); }); if (success) { - step(resultOrError); + return step(resultOrError); } else { - reject(resultOrError); + return reject(resultOrError); } }); } export function __TS__Await(this: void, thing: unknown) { - return coroutine.yield(thing); + return coyield(thing); } diff --git a/src/lualib/Promise.ts b/src/lualib/Promise.ts index 3ba075326..4ca124e97 100644 --- a/src/lualib/Promise.ts +++ b/src/lualib/Promise.ts @@ -9,111 +9,113 @@ export const enum PromiseState { Rejected, } -type FulfillCallback = (value: TData) => TResult | PromiseLike; -type RejectCallback = (reason: any) => TResult | PromiseLike; - -function promiseDeferred() { - let resolve: FulfillCallback; - let reject: RejectCallback; - const promise = new Promise((res, rej) => { +type PromiseExecutor = ConstructorParameters>[0]; +type PromiseResolve = Parameters>[0]; +type PromiseReject = Parameters>[1]; +type PromiseResolveCallback = (value: TValue) => TResult | PromiseLike; +type PromiseRejectCallback = (reason: any) => TResult | PromiseLike; + +function makeDeferredPromiseFactory(this: void) { + let resolve: PromiseResolve; + let reject: PromiseReject; + const executor: PromiseExecutor = (res, rej) => { resolve = res; reject = rej; - }); - - // @ts-ignore This is alright because TS doesnt understand the callback will immediately be called - return { promise, resolve, reject }; + }; + return function (this: void) { + const promise = new Promise(executor); + return $multi(promise, resolve, reject); + }; } -function isPromiseLike(thing: unknown): thing is PromiseLike { - return thing instanceof __TS__Promise; +const makeDeferredPromise = makeDeferredPromiseFactory(); + +function isPromiseLike(this: void, value: unknown): value is PromiseLike { + return value instanceof __TS__Promise; } +function doNothing(): void {} + +const pcall = _G.pcall; + export class __TS__Promise implements Promise { public state = PromiseState.Pending; public value?: T; public rejectionReason?: any; - private fulfilledCallbacks: Array> = []; - private rejectedCallbacks: Array> = []; + private fulfilledCallbacks: Array> = []; + private rejectedCallbacks: PromiseReject[] = []; private finallyCallbacks: Array<() => void> = []; // @ts-ignore public [Symbol.toStringTag]: string; // Required to implement interface, no output Lua // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/resolve - public static resolve(this: void, data: TData): Promise { + public static resolve(this: void, value: T | PromiseLike): Promise> { + if (value instanceof __TS__Promise) { + return value; + } // Create and return a promise instance that is already resolved - const promise = new __TS__Promise(() => {}); + const promise = new __TS__Promise>(doNothing); promise.state = PromiseState.Fulfilled; - promise.value = data; + promise.value = value as Awaited; return promise; } // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/reject - public static reject(this: void, reason: any): Promise { + public static reject(this: void, reason?: any): Promise { // Create and return a promise instance that is already rejected - const promise = new __TS__Promise(() => {}); + const promise = new __TS__Promise(doNothing); promise.state = PromiseState.Rejected; promise.rejectionReason = reason; return promise; } - constructor(executor: (resolve: (data: T) => void, reject: (reason: any) => void) => void) { - try { - executor(this.resolve.bind(this), this.reject.bind(this)); - } catch (e) { + constructor(executor: PromiseExecutor) { + // Avoid unnecessary local functions allocations by using `pcall` explicitly + const [success, error] = pcall( + executor, + undefined, + v => this.resolve(v), + err => this.reject(err) + ); + if (!success) { // When a promise executor throws, the promise should be rejected with the thrown object as reason - this.reject(e); + this.reject(error); } } // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/then public then( - onFulfilled?: FulfillCallback, - onRejected?: RejectCallback + onFulfilled?: PromiseResolveCallback, + onRejected?: PromiseRejectCallback ): Promise { - const { promise, resolve, reject } = promiseDeferred(); - - const isFulfilled = this.state === PromiseState.Fulfilled; - const isRejected = this.state === PromiseState.Rejected; - - if (onFulfilled) { - const internalCallback = this.createPromiseResolvingCallback(onFulfilled, resolve, reject); - this.fulfilledCallbacks.push(internalCallback); + const [promise, resolve, reject] = makeDeferredPromise(); - if (isFulfilled) { - // If promise already resolved, immediately call callback - internalCallback(this.value!); - } - } else { + this.addCallbacks( // We always want to resolve our child promise if this promise is resolved, even if we have no handler - this.fulfilledCallbacks.push(v => resolve(v)); - } - - if (onRejected) { - const internalCallback = this.createPromiseResolvingCallback(onRejected, resolve, reject); - this.rejectedCallbacks.push(internalCallback); - - if (isRejected) { - // If promise already rejected, immediately call callback - internalCallback(this.rejectionReason); - } - } else { + onFulfilled ? this.createPromiseResolvingCallback(onFulfilled, resolve, reject) : resolve, // We always want to reject our child promise if this promise is rejected, even if we have no handler - this.rejectedCallbacks.push(err => reject(err)); - } + onRejected ? this.createPromiseResolvingCallback(onRejected, resolve, reject) : reject + ); - if (isFulfilled) { - // If promise already resolved, also resolve returned promise - resolve(this.value!); - } + return promise as Promise; + } - if (isRejected) { - // If promise already rejected, also reject returned promise - reject(this.rejectionReason); + // Both callbacks should never throw! + public addCallbacks(fulfilledCallback: (value: T) => void, rejectedCallback: (rejectionReason: any) => void): void { + if (this.state === PromiseState.Fulfilled) { + // If promise already resolved, immediately call callback. We don't even need to store rejected callback + // Tail call return is important! + return fulfilledCallback(this.value!); + } + if (this.state === PromiseState.Rejected) { + // Similar thing + return rejectedCallback(this.rejectionReason); } - return promise as Promise; + this.fulfilledCallbacks.push(fulfilledCallback as any); + this.rejectedCallbacks.push(rejectedCallback); } // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/catch @@ -134,26 +136,22 @@ export class __TS__Promise implements Promise { return this; } - private resolve(data: T): void { - if (data instanceof __TS__Promise) { - data.then( + private resolve(value: T | PromiseLike): void { + if (isPromiseLike(value)) { + // Tail call return is important! + return (value as __TS__Promise).addCallbacks( v => this.resolve(v), err => this.reject(err) ); - return; } // Resolve this promise, if it is still pending. This function is passed to the constructor function. if (this.state === PromiseState.Pending) { this.state = PromiseState.Fulfilled; - this.value = data; + this.value = value; - for (const callback of this.fulfilledCallbacks) { - callback(data); - } - for (const callback of this.finallyCallbacks) { - callback(); - } + // Tail call return is important! + return this.invokeCallbacks(this.fulfilledCallbacks, value); } } @@ -163,55 +161,85 @@ export class __TS__Promise implements Promise { this.state = PromiseState.Rejected; this.rejectionReason = reason; - for (const callback of this.rejectedCallbacks) { - callback(reason); + // Tail call return is important! + return this.invokeCallbacks(this.rejectedCallbacks, reason); + } + } + + private invokeCallbacks(callbacks: ReadonlyArray<(value: T) => void>, value: T): void { + const callbacksLength = callbacks.length; + const finallyCallbacks = this.finallyCallbacks; + const finallyCallbacksLength = finallyCallbacks.length; + + if (callbacksLength !== 0) { + for (const i of $range(1, callbacksLength - 1)) { + callbacks[i - 1](value); } - for (const callback of this.finallyCallbacks) { - callback(); + // Tail call optimization for a common case. + if (finallyCallbacksLength === 0) { + return callbacks[callbacksLength - 1](value); } + callbacks[callbacksLength - 1](value); + } + + if (finallyCallbacksLength !== 0) { + for (const i of $range(1, finallyCallbacksLength - 1)) { + finallyCallbacks[i - 1](); + } + return finallyCallbacks[finallyCallbacksLength - 1](); } } private createPromiseResolvingCallback( - f: FulfillCallback | RejectCallback, - resolve: FulfillCallback, - reject: RejectCallback + f: PromiseResolveCallback | PromiseRejectCallback, + resolve: (data: TResult1 | TResult2) => void, + reject: (reason: any) => void ) { - return (value: T) => { - try { - this.handleCallbackData(f(value), resolve, reject); - } catch (e) { - // If a handler function throws an error, the promise returned by then gets rejected with the thrown error as its value - reject(e); + return (value: T): void => { + const [success, resultOrError] = pcall< + undefined, + [T], + TResult1 | PromiseLike | TResult2 | PromiseLike + >(f, undefined, value); + if (!success) { + // Tail call return is important! + return reject(resultOrError); } + // Tail call return is important! + return this.handleCallbackValue(resultOrError, resolve, reject); }; } - private handleCallbackData( - data: TResult | PromiseLike, - resolve: FulfillCallback, - reject: RejectCallback - ) { - if (isPromiseLike(data)) { - const nextpromise = data as __TS__Promise; + private handleCallbackValue( + value: TResult | PromiseLike, + resolve: (data: TResult1 | TResult2) => void, + reject: (reason: any) => void + ): void { + if (isPromiseLike(value)) { + const nextpromise = value as __TS__Promise; if (nextpromise.state === PromiseState.Fulfilled) { // If a handler function returns an already fulfilled promise, - // the promise returned by then gets fulfilled with that promise's value - resolve(nextpromise.value!); + // the promise returned by then gets fulfilled with that promise's value. + // Tail call return is important! + return resolve(nextpromise.value!); } else if (nextpromise.state === PromiseState.Rejected) { // If a handler function returns an already rejected promise, - // the promise returned by then gets fulfilled with that promise's value - reject(nextpromise.rejectionReason); + // the promise returned by then gets fulfilled with that promise's value. + // Tail call return is important! + return reject(nextpromise.rejectionReason); } else { // If a handler function returns another pending promise object, the resolution/rejection // of the promise returned by then will be subsequent to the resolution/rejection of // the promise returned by the handler. - data.then(resolve, reject); + // We cannot use `then` because we need to do tail call, and `then` returns a Promise. + // `resolve` and `reject` should never throw. + return nextpromise.addCallbacks(resolve, reject); } } else { // If a handler returns a value, the promise returned by then gets resolved with the returned value as its value // If a handler doesn't return anything, the promise returned by then gets resolved with undefined - resolve(data); + // Tail call return is important! + return resolve(value); } } } diff --git a/test/unit/builtins/async-await.spec.ts b/test/unit/builtins/async-await.spec.ts index 3dbb77abe..45adb4518 100644 --- a/test/unit/builtins/async-await.spec.ts +++ b/test/unit/builtins/async-await.spec.ts @@ -24,6 +24,31 @@ function defer() { return { promise, resolve, reject }; }`; +test("high amount of chained awaits doesn't cause stack overflow", () => { + util.testFunction` + let result = "not executed"; + + async function delay() { + return; + } + + async function loop() { + try { + for (let i = 0; i < 500000; ++i) { + await delay(); + } + result = "success"; + } catch (e) { + result = e; + } + } + + loop(); + + return result; + `.expectToEqual("success"); +}); + test("can await already resolved promise", () => { util.testFunction` const result = []; diff --git a/test/unit/builtins/promise.spec.ts b/test/unit/builtins/promise.spec.ts index fa6f6cf0f..cb18db994 100644 --- a/test/unit/builtins/promise.spec.ts +++ b/test/unit/builtins/promise.spec.ts @@ -839,6 +839,26 @@ test("resolving promise with pending promise will keep pending until promise2 re .expectToEqual(["promise 2", "rejection", "promise 1", "rejection"]); }); +describe("Promise.resolve", () => { + test("returns the given resolved promise", () => { + util.testFunction` + let result = 0; + Promise.resolve(Promise.resolve(4)) + .then((value) => result = value); + return result; + `.expectToEqual(4); + }); + + test("returns the given rejected promise", () => { + util.testFunction` + let result = 0; + Promise.resolve(Promise.reject(4)) + .catch((value) => result = value); + return result; + `.expectToEqual(4); + }); +}); + describe("Promise.all", () => { test("resolves once all arguments are resolved", () => { util.testFunction` From 3fa738ca587e0a4e16f68765cc507c80fa3ec2ce Mon Sep 17 00:00:00 2001 From: Rhazarian Date: Sun, 4 Feb 2024 14:18:07 +0100 Subject: [PATCH 2/2] remove unneeded adopt --- src/lualib/Await.ts | 9 ++------- src/lualib/Promise.ts | 4 ++-- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/lualib/Await.ts b/src/lualib/Await.ts index ea1ade3ac..f5c368619 100644 --- a/src/lualib/Await.ts +++ b/src/lualib/Await.ts @@ -21,11 +21,6 @@ const coresume = coroutine.resume; const costatus = coroutine.status; const coyield = coroutine.yield; -// The precise implementation type is important here because we use special functions due to necessary optimizations -function adopt(this: void, value: unknown): __TS__Promise { - return value instanceof __TS__Promise ? value : (__TS__Promise.resolve(value) as __TS__Promise); -} - // Be extremely careful editing this function. A single non-tail function call may ruin chained awaits performance // eslint-disable-next-line @typescript-eslint/promise-function-async export function __TS__AsyncAwaiter(this: void, generator: (this: void) => void) { @@ -53,12 +48,12 @@ export function __TS__AsyncAwaiter(this: void, generator: (this: void) => void) } // We cannot use `then` because we need to avoid calling `coroutine.resume` from inside `pcall` // `fulfilled` and `reject` should never throw. Tail call return is important! - return adopt(result).addCallbacks(fulfilled, reject); + return __TS__Promise.resolve(result).addCallbacks(fulfilled, reject); } const [success, resultOrError] = coresume(asyncCoroutine, (v: unknown) => { resolved = true; - return adopt(v).addCallbacks(resolve, reject); + return __TS__Promise.resolve(v).addCallbacks(resolve, reject); }); if (success) { return step(resultOrError); diff --git a/src/lualib/Promise.ts b/src/lualib/Promise.ts index 4ca124e97..c5b6aa2a5 100644 --- a/src/lualib/Promise.ts +++ b/src/lualib/Promise.ts @@ -51,7 +51,7 @@ export class __TS__Promise implements Promise { public [Symbol.toStringTag]: string; // Required to implement interface, no output Lua // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/resolve - public static resolve(this: void, value: T | PromiseLike): Promise> { + public static resolve(this: void, value: T | PromiseLike): __TS__Promise> { if (value instanceof __TS__Promise) { return value; } @@ -63,7 +63,7 @@ export class __TS__Promise implements Promise { } // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/reject - public static reject(this: void, reason?: any): Promise { + public static reject(this: void, reason?: any): __TS__Promise { // Create and return a promise instance that is already rejected const promise = new __TS__Promise(doNothing); promise.state = PromiseState.Rejected;