From e86d7cab06095f5c61d6b97f731ea903b80ef515 Mon Sep 17 00:00:00 2001 From: Luke Wilde Date: Fri, 14 Jul 2023 22:23:43 +0100 Subject: [PATCH] LibJS: Don't crash on broken promises in AsyncGenerator#return See: https://github.com/tc39/ecma262/pull/2683 --- .../LibJS/Runtime/AsyncGenerator.cpp | 46 ++++++++++++++----- .../Libraries/LibJS/Runtime/AsyncGenerator.h | 2 +- .../LibJS/Runtime/AsyncGeneratorPrototype.cpp | 4 +- .../AsyncGenerator.prototype.return.js | 21 +++++++++ 4 files changed, 58 insertions(+), 15 deletions(-) diff --git a/Userland/Libraries/LibJS/Runtime/AsyncGenerator.cpp b/Userland/Libraries/LibJS/Runtime/AsyncGenerator.cpp index 6c0edd2385..18a7a57412 100644 --- a/Userland/Libraries/LibJS/Runtime/AsyncGenerator.cpp +++ b/Userland/Libraries/LibJS/Runtime/AsyncGenerator.cpp @@ -342,7 +342,8 @@ ThrowCompletionOr AsyncGenerator::resume(VM& vm, Completion completion) } // 27.6.3.9 AsyncGeneratorAwaitReturn ( generator ), https://tc39.es/ecma262/#sec-asyncgeneratorawaitreturn -ThrowCompletionOr AsyncGenerator::await_return() +// With unmerged broken promise fixup from https://github.com/tc39/ecma262/pull/2683 +void AsyncGenerator::await_return() { auto& vm = this->vm(); auto& realm = *vm.current_realm(); @@ -362,10 +363,31 @@ ThrowCompletionOr AsyncGenerator::await_return() // 5. Assert: completion.[[Type]] is return. VERIFY(completion.type() == Completion::Type::Return); - // 6. Let promise be ? PromiseResolve(%Promise%, completion.[[Value]]). - auto* promise = TRY(promise_resolve(vm, realm.intrinsics().promise_constructor(), completion.value().value())); + // 6. Let promiseCompletion be Completion(PromiseResolve(%Promise%, _completion_.[[Value]])). + auto promise_completion = promise_resolve(vm, realm.intrinsics().promise_constructor(), completion.value().value()); - // 7. Let fulfilledClosure be a new Abstract Closure with parameters (value) that captures generator and performs + // 7. If promiseCompletion is an abrupt completion, then + if (promise_completion.is_throw_completion()) { + // a. Set generator.[[AsyncGeneratorState]] to completed. + m_async_generator_state = State::Completed; + + // b. Perform AsyncGeneratorCompleteStep(generator, promiseCompletion, true). + complete_step(promise_completion.release_error(), true); + + // c. Perform AsyncGeneratorDrainQueue(generator). + drain_queue(); + + // d. Return unused. + return; + } + + // 8. Assert: promiseCompletion.[[Type]] is normal. + VERIFY(!promise_completion.is_throw_completion()); + + // 9. Let promise be promiseCompletion.[[Value]]. + auto* promise = promise_completion.release_value(); + + // 10. Let fulfilledClosure be a new Abstract Closure with parameters (value) that captures generator and performs // the following steps when called: auto fulfilled_closure = [this](VM& vm) -> ThrowCompletionOr { // a. Set generator.[[AsyncGeneratorState]] to completed. @@ -384,10 +406,10 @@ ThrowCompletionOr AsyncGenerator::await_return() return js_undefined(); }; - // 8. Let onFulfilled be CreateBuiltinFunction(fulfilledClosure, 1, "", « »). + // 11. Let onFulfilled be CreateBuiltinFunction(fulfilledClosure, 1, "", « »). auto on_fulfilled = NativeFunction::create(realm, move(fulfilled_closure), 1, ""); - // 9. Let rejectedClosure be a new Abstract Closure with parameters (reason) that captures generator and performs + // 12. Let rejectedClosure be a new Abstract Closure with parameters (reason) that captures generator and performs // the following steps when called: auto rejected_closure = [this](VM& vm) -> ThrowCompletionOr { // a. Set generator.[[AsyncGeneratorState]] to completed. @@ -406,17 +428,17 @@ ThrowCompletionOr AsyncGenerator::await_return() return js_undefined(); }; - // 10. Let onRejected be CreateBuiltinFunction(rejectedClosure, 1, "", « »). + // 13. Let onRejected be CreateBuiltinFunction(rejectedClosure, 1, "", « »). auto on_rejected = NativeFunction::create(realm, move(rejected_closure), 1, ""); - // 11. Perform PerformPromiseThen(promise, onFulfilled, onRejected). + // 14. Perform PerformPromiseThen(promise, onFulfilled, onRejected). // NOTE: await_return should only be called when the generator is in SuspendedStart or Completed state, // so an await shouldn't be running currently, so it should be safe to overwrite m_current_promise. m_current_promise = verify_cast(promise); m_current_promise->perform_then(on_fulfilled, on_rejected, {}); - // 12. Return unused. - return {}; + // 15. Return unused. + return; } // 27.6.3.5 AsyncGeneratorCompleteStep ( generator, completion, done [ , realm ] ), https://tc39.es/ecma262/#sec-asyncgeneratorcompletestep @@ -507,8 +529,8 @@ void AsyncGenerator::drain_queue() // i. Set generator.[[AsyncGeneratorState]] to awaiting-return. m_async_generator_state = State::AwaitingReturn; - // ii. Perform ! AsyncGeneratorAwaitReturn(generator). - MUST(await_return()); + // ii. Perform AsyncGeneratorAwaitReturn(generator). + await_return(); // iii. Set done to true. done = true; diff --git a/Userland/Libraries/LibJS/Runtime/AsyncGenerator.h b/Userland/Libraries/LibJS/Runtime/AsyncGenerator.h index 7e3544e187..933fe199c6 100644 --- a/Userland/Libraries/LibJS/Runtime/AsyncGenerator.h +++ b/Userland/Libraries/LibJS/Runtime/AsyncGenerator.h @@ -33,7 +33,7 @@ public: void async_generator_enqueue(Completion, NonnullGCPtr); ThrowCompletionOr resume(VM&, Completion completion); - ThrowCompletionOr await_return(); + void await_return(); void complete_step(Completion, bool done, Realm* realm = nullptr); void drain_queue(); diff --git a/Userland/Libraries/LibJS/Runtime/AsyncGeneratorPrototype.cpp b/Userland/Libraries/LibJS/Runtime/AsyncGeneratorPrototype.cpp index be33b0b17b..0fad0e3c1b 100644 --- a/Userland/Libraries/LibJS/Runtime/AsyncGeneratorPrototype.cpp +++ b/Userland/Libraries/LibJS/Runtime/AsyncGeneratorPrototype.cpp @@ -132,8 +132,8 @@ JS_DEFINE_NATIVE_FUNCTION(AsyncGeneratorPrototype::return_) // a. Set generator.[[AsyncGeneratorState]] to awaiting-return. generator->set_async_generator_state({}, AsyncGenerator::State::AwaitingReturn); - // b. Perform ! AsyncGeneratorAwaitReturn(generator). - MUST(generator->await_return()); + // b. Perform AsyncGeneratorAwaitReturn(generator). + generator->await_return(); } // 9. Else if state is suspendedYield, then else if (state == AsyncGenerator::State::SuspendedYield) { diff --git a/Userland/Libraries/LibJS/Tests/builtins/AsyncGenerator/AsyncGenerator.prototype.return.js b/Userland/Libraries/LibJS/Tests/builtins/AsyncGenerator/AsyncGenerator.prototype.return.js index dabba46645..cbb9c8f416 100644 --- a/Userland/Libraries/LibJS/Tests/builtins/AsyncGenerator/AsyncGenerator.prototype.return.js +++ b/Userland/Libraries/LibJS/Tests/builtins/AsyncGenerator/AsyncGenerator.prototype.return.js @@ -143,4 +143,25 @@ describe("errors", () => { expect(rejection).toBeInstanceOf(TypeError); expect(rejection.message).toBe("Not an object of type AsyncGenerator"); }); + + // https://github.com/tc39/ecma262/pull/2683 + test("doesn't crash on broken promises", () => { + const promise = Promise.resolve(1337); + Object.defineProperty(promise, "constructor", { + get: function () { + throw new Error("yaksplode"); + }, + }); + + async function* generator() {} + const generatorObject = generator(); + + let rejection = null; + generatorObject.return(promise).catch(error => { + rejection = error; + }); + runQueuedPromiseJobs(); + expect(rejection).toBeInstanceOf(Error); + expect(rejection.message).toBe("yaksplode"); + }); });