Commit 3f366186 authored by littledan's avatar littledan Committed by Commit bot

Revert of Fix async/await memory leak (patchset #5 id:160001 of...

Revert of Fix async/await memory leak (patchset #5 id:160001 of https://codereview.chromium.org/2348403002/ )

Reason for revert:
Still causes issues on bot (sometimes!)

Original issue's description:
> Reland of Fix async/await memory leak (patchset #1 id:1 of https://codereview.chromium.org/2354473002/ )
>
> Reason for revert:
> Relanding with faster-running test
>
> Original issue's description:
> > Revert of Fix async/await memory leak (patchset #5 id:80001 of https://codereview.chromium.org/2334323006/ )
> >
> > Reason for revert:
> > newly introduced test async-await-loop times out: https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20arm64%20-%20sim%20-%20MSAN/builds/10894/steps/Ignition%20-%20turbofan%20%28flakes%29/logs/async-await-loop
> >
> > Original issue's description:
> > > Fix async/await memory leak
> > >
> > > This patch closes a memory leak in async/await where the desugaring
> > > was creating a situation analagous to that described in v8:5002.
> > > Intermediate Promises were being kept alive, so a long-running loop
> > > would cause linear memory usage on the heap. This patch returns
> > > undefined to the 'then' callback passed into PerformPromiseThen
> > > in order to avoid this hazard. Test expectations are fixed to remove
> > > expecting extraneous events which occurred on Promises that are
> > > now not given unnecessarily complex resolution paths before being
> > > thrown away.
> > >
> > > BUG=v8:5390
> > >
> > > Committed: https://crrev.com/a0ba18e9634c5e2d439033ab61a77cff54f9af35
> > > Cr-Commit-Position: refs/heads/master@{#39479}
> >
> > TBR=adamk@chromium.org,caitp@igalia.com,littledan@chromium.org
> > NOTRY=true
> > BUG=v8:5390
> >
> > Committed: https://crrev.com/196db1999da130019bbf8e3bd65977f840e8afaf
> > Cr-Commit-Position: refs/heads/master@{#39493}
>
> TBR=adamk@chromium.org,caitp@igalia.com,hablich@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> BUG=v8:5390
>
> Committed: https://crrev.com/e51482f01f26e0013e6377e85c4d2c41900e403c
> Cr-Commit-Position: refs/heads/master@{#39508}

TBR=adamk@chromium.org,caitp@igalia.com,hablich@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:5390

Review-Url: https://codereview.chromium.org/2348403003
Cr-Commit-Position: refs/heads/master@{#39512}
parent cb19257a
...@@ -66,22 +66,10 @@ function AsyncFunctionAwait(generator, awaited, mark) { ...@@ -66,22 +66,10 @@ function AsyncFunctionAwait(generator, awaited, mark) {
// ); // );
var promise = PromiseCastResolved(awaited); var promise = PromiseCastResolved(awaited);
var onFulfilled = sentValue => { var onFulfilled =
%_Call(AsyncFunctionNext, generator, sentValue); (sentValue) => %_Call(AsyncFunctionNext, generator, sentValue);
// The resulting Promise is a throwaway, so it doesn't matter what it var onRejected =
// resolves to. What is important is that we don't end up keeping the (sentError) => %_Call(AsyncFunctionThrow, generator, sentError);
// whole chain of intermediate Promises alive by returning the value
// of AsyncFunctionNext, as that would create a memory leak.
return;
};
var onRejected = sentError => {
%_Call(AsyncFunctionThrow, generator, sentError);
// Similarly, returning the huge Promise here would cause a long
// resolution chain to find what the exception to throw is, and
// create a similar memory leak, and it does not matter what
// sort of rejection this intermediate Promise becomes.
return;
}
if (mark && DEBUG_IS_ACTIVE && IsPromise(awaited)) { if (mark && DEBUG_IS_ACTIVE && IsPromise(awaited)) {
// Mark the reject handler callback such that it does not influence // Mark the reject handler callback such that it does not influence
......
...@@ -4560,46 +4560,21 @@ Expression* Parser::ExpressionListToExpression(ZoneList<Expression*>* args) { ...@@ -4560,46 +4560,21 @@ Expression* Parser::ExpressionListToExpression(ZoneList<Expression*>* args) {
Expression* Parser::RewriteAwaitExpression(Expression* value, int await_pos) { Expression* Parser::RewriteAwaitExpression(Expression* value, int await_pos) {
// yield do { // yield do {
// promise_tmp = .promise;
// tmp = <operand>; // tmp = <operand>;
// %AsyncFunctionAwait(.generator_object, tmp); // tmp = %AsyncFunctionAwait(.generator_object, tmp);
// promise_tmp
// } // }
// The value of the expression is returned to the caller of the async
// function for the first yield statement; for this, .promise is the
// appropriate return value, being a Promise that will be fulfilled or
// rejected with the appropriate value by the desugaring. Subsequent yield
// occurrences will return to the AsyncFunctionNext call within the
// implemementation of the intermediate throwaway Promise's then handler.
// This handler has nothing useful to do with the value, as the Promise is
// ignored. If we yielded the value of the throwawayPromise that
// AsyncFunctionAwait creates as an intermediate, it would create a memory
// leak; we must return .promise instead;
// The operand needs to be evaluated on a separate statement in order to get
// a break location, and the .promise needs to be read earlier so that it
// doesn't insert a false location.
// TODO(littledan): investigate why this ordering is needed in more detail.
Variable* generator_object_variable = Variable* generator_object_variable =
function_state_->generator_object_variable(); function_state_->generator_object_variable();
// If generator_object_variable is null, // If generator_object_variable is null,
// TODO(littledan): Is this necessary?
if (!generator_object_variable) return value; if (!generator_object_variable) return value;
const int nopos = kNoSourcePosition; const int nopos = kNoSourcePosition;
Block* do_block = factory()->NewBlock(nullptr, 3, false, nopos); Variable* temp_var = NewTemporary(ast_value_factory()->empty_string());
Block* do_block = factory()->NewBlock(nullptr, 2, false, nopos);
Variable* promise_temp_var =
NewTemporary(ast_value_factory()->empty_string());
Expression* promise_assignment = factory()->NewAssignment(
Token::ASSIGN, factory()->NewVariableProxy(promise_temp_var),
BuildDotPromise(), nopos);
do_block->statements()->Add(
factory()->NewExpressionStatement(promise_assignment, nopos), zone());
// Wrap value evaluation to provide a break location. // Wrap value evaluation to provide a break location.
Variable* temp_var = NewTemporary(ast_value_factory()->empty_string());
Expression* value_assignment = factory()->NewAssignment( Expression* value_assignment = factory()->NewAssignment(
Token::ASSIGN, factory()->NewVariableProxy(temp_var), value, nopos); Token::ASSIGN, factory()->NewVariableProxy(temp_var), value, nopos);
do_block->statements()->Add( do_block->statements()->Add(
...@@ -4619,13 +4594,14 @@ Expression* Parser::RewriteAwaitExpression(Expression* value, int await_pos) { ...@@ -4619,13 +4594,14 @@ Expression* Parser::RewriteAwaitExpression(Expression* value, int await_pos) {
Expression* async_function_await = Expression* async_function_await =
factory()->NewCallRuntime(Context::ASYNC_FUNCTION_AWAIT_CAUGHT_INDEX, factory()->NewCallRuntime(Context::ASYNC_FUNCTION_AWAIT_CAUGHT_INDEX,
async_function_await_args, nopos); async_function_await_args, nopos);
do_block->statements()->Add(
factory()->NewExpressionStatement(async_function_await, await_pos),
zone());
// Wrap await to provide a break location between value evaluation and yield. // Wrap await to provide a break location between value evaluation and yield.
Expression* do_expr = Expression* await_assignment = factory()->NewAssignment(
factory()->NewDoExpression(do_block, promise_temp_var, nopos); Token::ASSIGN, factory()->NewVariableProxy(temp_var),
async_function_await, nopos);
do_block->statements()->Add(
factory()->NewExpressionStatement(await_assignment, await_pos), zone());
Expression* do_expr = factory()->NewDoExpression(do_block, temp_var, nopos);
generator_object = factory()->NewVariableProxy(generator_object_variable); generator_object = factory()->NewVariableProxy(generator_object_variable);
return factory()->NewYield(generator_object, do_expr, nopos, return factory()->NewYield(generator_object, do_expr, nopos,
......
// Copyright 2016 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: --harmony-async-await --allow-natives-syntax --max-old-space-size 3
let out;
async function longLoop() {
for (let i = 0; i < 8000; i++) await undefined;
out = 1;
}
longLoop();
%RunMicrotasks();
assertEquals(1, out);
// Copyright 2016 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: --harmony-async-await --allow-natives-syntax --max-old-space-size 3
let out;
async function thrower() { throw undefined; }
async function throwLoop() {
for (let i = 0; i < 8000; i++) {
try { await thrower(); }
catch (e) {}
}
out = 2;
}
throwLoop();
%RunMicrotasks();
assertEquals(2, out);
...@@ -9,14 +9,20 @@ Debug = debug.Debug; ...@@ -9,14 +9,20 @@ Debug = debug.Debug;
var base_id = -1; var base_id = -1;
var exception = null; var exception = null;
var expected = [ var expected = [
'enqueue #1', "enqueue #1",
'willHandle #1', "willHandle #1",
'then #1', "then #1",
'enqueue #2', "enqueue #2",
'didHandle #1', "enqueue #3",
'willHandle #2', "didHandle #1",
'then #2', "willHandle #2",
'didHandle #2', "then #2",
"didHandle #2",
"willHandle #3",
"enqueue #4",
"didHandle #3",
"willHandle #4",
"didHandle #4",
]; ];
function assertLog(msg) { function assertLog(msg) {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment