backport ERM fixes (#19)

denoland · Feb 21, 2025 · f7e60d5 · f7e60d5
1 parent db818c9
commit f7e60d5
Show file tree

Hide file tree

Showing 2 changed files with 221 additions and 0 deletions.
diff --git a/patches/0005-explicit-resource-management-Call-dispose-method-on-.patch b/patches/0005-explicit-resource-management-Call-dispose-method-on-.patch
@@ -0,0 +1,86 @@
+From c43f1abb087f90d728ace458df99f10d55442254 Mon Sep 17 00:00:00 2001
+From: Rezvan Mahdavi Hezaveh <[email protected]>
+Date: Fri, 14 Feb 2025 22:27:19 +0000
+Subject: [PATCH] [explicit-resource-management] Call dispose method on correct
+ receiver
+
+This CL fixes the bug of calling dispose method on undefined receiver.
+
+Bug: 396661134
+Change-Id: I0121318ddc3afe61867ca34625061ba9a67db9b9
+Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6271683
+Reviewed-by: Shu-yu Guo <[email protected]>
+Commit-Queue: Rezvan Mahdavi Hezaveh <[email protected]>
+Cr-Commit-Position: refs/heads/main@{#98712}
+---
+ .../builtins-async-disposable-stack.cc        |  5 +--
+ ...anagement-call-sync-from-async-receiver.js | 34 +++++++++++++++++++
+ 2 files changed, 37 insertions(+), 2 deletions(-)
+ create mode 100644 test/mjsunit/harmony/explicit-resource-management-call-sync-from-async-receiver.js
+
+diff --git a/src/builtins/builtins-async-disposable-stack.cc b/src/builtins/builtins-async-disposable-stack.cc
+index 777e6551eb3..a1f2b557a25 100644
+--- a/src/builtins/builtins-async-disposable-stack.cc
++++ b/src/builtins/builtins-async-disposable-stack.cc
+@@ -77,6 +77,7 @@ BUILTIN(AsyncDisposeFromSyncDispose) {
+   //      captures method and performs the following steps when called:
+   //        a. Let O be the this value.
+   //        b. Let promiseCapability be ! NewPromiseCapability(%Promise%).
++  DirectHandle<Object> receiver = args.receiver();
+   DirectHandle<JSPromise> promise = isolate->factory()->NewJSPromise();
+
+   //        c. Let result be Completion(Call(method, O)).
+@@ -90,8 +91,8 @@ BUILTIN(AsyncDisposeFromSyncDispose) {
+   try_catch.SetVerbose(false);
+   try_catch.SetCaptureMessage(false);
+
+-  MaybeDirectHandle<Object> result = Execution::Call(
+-      isolate, sync_method, isolate->factory()->undefined_value(), {});
++  MaybeDirectHandle<Object> result =
++      Execution::Call(isolate, sync_method, receiver, {});
+
+   DirectHandle<Object> result_handle;
+
+diff --git a/test/mjsunit/harmony/explicit-resource-management-call-sync-from-async-receiver.js b/test/mjsunit/harmony/explicit-resource-management-call-sync-from-async-receiver.js
+new file mode 100644
+index 00000000000..d31cf5a7ac9
+--- /dev/null
++++ b/test/mjsunit/harmony/explicit-resource-management-call-sync-from-async-receiver.js
+@@ -0,0 +1,34 @@
++// Copyright 2025 the V8 project authors. All rights reserved.
++// Use of this source code is governed by a BSD-style license that can be
++// found in the LICENSE file.
++
++// Flags: --js-staging
++
++let value = false;
++
++const obj =
++    {
++      [Symbol.dispose]() {
++        value = (this === obj)
++      }
++    }
++
++async function TestAwaitUsing() {
++  await using x = obj;
++}
++
++async function TestAsyncDisposableStack() {
++  const stack = new AsyncDisposableStack();
++  stack.use(obj);
++  stack[Symbol.asyncDispose]();
++}
++
++async function RunTest() {
++  await TestAwaitUsing();
++  assertSame(true, value);
++  value = false;
++  await TestAsyncDisposableStack();
++  assertSame(true, value);
++}
++
++RunTest();
+-- 
+2.39.5 (Apple Git-154)
+
diff --git a/patches/0006-explicit-resource-management-Fix-await-using-in-top-.patch b/patches/0006-explicit-resource-management-Fix-await-using-in-top-.patch
@@ -0,0 +1,135 @@
+From b80573ff72d219dbcd44cd2054d2e7ecd6ef5b28 Mon Sep 17 00:00:00 2001
+From: Rezvan Mahdavi Hezaveh <[email protected]>
+Date: Wed, 19 Feb 2025 18:54:56 +0000
+Subject: [PATCH] [explicit-resource-management] Fix await using in top level
+ module
+
+This CL adds one suspend point in any scope that has at least one
+`await using` in it.
+
+Bug: 396661138
+Change-Id: I144a0ca49395b34309e7e35f070c98cc145af34b
+Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6279751
+Commit-Queue: Rezvan Mahdavi Hezaveh <[email protected]>
+Reviewed-by: Shu-yu Guo <[email protected]>
+Cr-Commit-Position: refs/heads/main@{#98811}
+---
+ src/parsing/parser-base.h                     | 28 ++++---------------
+ .../await-using-in-top-level-module.js        | 14 ++++++++++
+ 2 files changed, 20 insertions(+), 22 deletions(-)
+ create mode 100644 test/test262/local-tests/test/staging/explicit-resource-management/await-using-in-top-level-module.js
+
+diff --git a/src/parsing/parser-base.h b/src/parsing/parser-base.h
+index 95f8a28ccd5..83acb46aa4a 100644
+--- a/src/parsing/parser-base.h
++++ b/src/parsing/parser-base.h
+@@ -1206,17 +1206,6 @@ class ParserBase {
+          !scanner()->HasLineTerminatorAfterNextNext() &&
+          IfNextUsingKeyword(PeekAheadAhead())));
+   }
+-  FunctionState* AddOneSuspendPointIfBlockContainsAwaitUsing(
+-      Scope* scope, FunctionState* function_state) {
+-    if (scope->has_await_using_declaration()) {
+-      // Since, we handle async disposal of resources by promise chaining, just
+-      // one suspend point is needed at the end of the block that contains at
+-      // least one `await using`. This suspend point will be placed in the
+-      // `finally` block of rewritten block.
+-      function_state->AddSuspend();
+-    }
+-    return function_state;
+-  }
+   const PendingCompilationErrorHandler* pending_error_handler() const {
+     return pending_error_handler_;
+   }
+@@ -4437,9 +4426,12 @@ void ParserBase<Impl>::ParseVariableDeclarations(
+   parsing_result->descriptor.declaration_pos = peek_position();
+   parsing_result->descriptor.initialization_pos = peek_position();
+
++  Scope* target_scope = scope();
++
+   switch (peek()) {
+     case Token::kVar:
+       parsing_result->descriptor.mode = VariableMode::kVar;
++      target_scope = scope()->GetDeclarationScope();
+       Consume(Token::kVar);
+       break;
+     case Token::kConst:
+@@ -4475,6 +4467,9 @@ void ParserBase<Impl>::ParseVariableDeclarations(
+       DCHECK(!scanner()->HasLineTerminatorBeforeNext());
+       DCHECK(peek() != Token::kLeftBracket && peek() != Token::kLeftBrace);
+       parsing_result->descriptor.mode = VariableMode::kAwaitUsing;
++      if (!target_scope->has_await_using_declaration()) {
++        function_state_->AddSuspend();
++      }
+       break;
+     default:
+       UNREACHABLE();  // by current callers
+@@ -4483,9 +4478,6 @@ void ParserBase<Impl>::ParseVariableDeclarations(
+
+   VariableDeclarationParsingScope declaration(
+       impl(), parsing_result->descriptor.mode, names);
+-  Scope* target_scope = IsLexicalVariableMode(parsing_result->descriptor.mode)
+-                            ? scope()
+-                            : scope()->GetDeclarationScope();
+
+   auto declaration_it = target_scope->declarations()->end();
+
+@@ -4857,8 +4849,6 @@ void ParserBase<Impl>::ParseFunctionBody(
+         ParseStatementList(&inner_body, closing_token);
+         if (IsAsyncFunction(kind)) {
+           inner_scope->set_end_position(end_position());
+-          function_state_ = AddOneSuspendPointIfBlockContainsAwaitUsing(
+-              inner_scope, function_state_);
+         }
+       }
+       if (IsDerivedConstructor(kind)) {
+@@ -5682,8 +5672,6 @@ void ParserBase<Impl>::ParseStatementList(StatementListT* body,
+     if (stat->IsEmptyStatement()) continue;
+     body->Add(stat);
+   }
+-  function_state_ =
+-      AddOneSuspendPointIfBlockContainsAwaitUsing(scope(), function_state_);
+ }
+
+ template <typename Impl>
+@@ -5894,8 +5882,6 @@ typename ParserBase<Impl>::BlockT ParserBase<Impl>::ParseBlock(
+
+     impl()->RecordBlockSourceRange(body, end_pos);
+     body->set_scope(scope()->FinalizeBlockScope());
+-    function_state_ =
+-        AddOneSuspendPointIfBlockContainsAwaitUsing(scope(), function_state_);
+   }
+
+   body->InitializeStatements(statements, zone());
+@@ -6393,8 +6379,6 @@ typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseSwitchStatement(
+     scope()->set_end_position(end_pos);
+     impl()->RecordSwitchStatementSourceRange(switch_statement, end_pos);
+     Scope* switch_scope = scope()->FinalizeBlockScope();
+-    function_state_ =
+-        AddOneSuspendPointIfBlockContainsAwaitUsing(scope(), function_state_);
+     if (switch_scope != nullptr) {
+       return impl()->RewriteSwitchStatement(switch_statement, switch_scope);
+     }
+diff --git a/test/test262/local-tests/test/staging/explicit-resource-management/await-using-in-top-level-module.js b/test/test262/local-tests/test/staging/explicit-resource-management/await-using-in-top-level-module.js
+new file mode 100644
+index 00000000000..18c2bfc50e4
+--- /dev/null
++++ b/test/test262/local-tests/test/staging/explicit-resource-management/await-using-in-top-level-module.js
+@@ -0,0 +1,14 @@
++// Copyright (C) 2025 the V8 project authors. All rights reserved.
++// This code is governed by the BSD license found in the LICENSE file.
++
++/*---
++description: await using should be allowed in top-level module.
++flags: [module]
++features: [explicit-resource-management]
++---*/
++
++await using x = {
++    [Symbol.asyncDispose]() {
++      return 42;
++    },
++};
+-- 
+2.39.5 (Apple Git-154)
+