Fix async/await memory leak

src/parsing/parser.cc

 // Copyright 2012 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.
 
 #include "src/parsing/parser.h"
 
 #include <memory>
 
 #include "src/api.h"
 #include "src/ast/ast-expression-rewriter.h"
@@ skipping 4361 matching lines @@
   Expression* expr = args->at(0);
   for (int i = 1; i < args->length(); ++i) {
     expr = factory()->NewBinaryOperation(Token::COMMA, expr, args->at(i),
                                          expr->position());
   }
   return expr;
 }
 
 Expression* Parser::RewriteAwaitExpression(Expression* value, int await_pos) {
   // yield do {
+  //   promise_tmp = .promise;
   //   tmp = <operand>;
-  //   tmp = %AsyncFunctionAwait(.generator_object, tmp);
+  //   %AsyncFunctionAwait(.generator_object, tmp);
+  //   promise_tmp
   // }
+  // 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

[adamk, 2016/09/16 17:24:47]

The ".promise needs to be read earlier" I don't understand. How is the reading
of .promise exposed?

[Dan Ehrenberg, 2016/09/16 17:42:14]

This shows up in locations printed in the stack trace. Added a TODO to
investigate further. Anyway, we need to read it into a tmp to make it the return
value of the DoExpression.

+  // doesn't insert a false break location.
+  // 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;
   Variable* generator_object_variable =
       function_state_->generator_object_variable();
 
   // If generator_object_variable is null,
   if (!generator_object_variable) return value;
 
   const int nopos = kNoSourcePosition;
 
   Variable* temp_var = NewTemporary(ast_value_factory()->empty_string());
   Block* do_block = factory()->NewBlock(nullptr, 2, false, nopos);
 
   // Wrap value evaluation to provide a break location.
+  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, value->position()),

[adamk, 2016/09/16 17:24:47]

This looks weird to me, why does this have a non-kNoSourcePosition? Especially
weird since it's the same position used for copying the value below.

[Dan Ehrenberg, 2016/09/16 17:42:13]

Oops, somehow lost the change to make it nopos in a rebase. Fixed.

+      zone());
+
   Expression* value_assignment = factory()->NewAssignment(
       Token::ASSIGN, factory()->NewVariableProxy(temp_var), value, nopos);
   do_block->statements()->Add(
       factory()->NewExpressionStatement(value_assignment, value->position()),
       zone());
 
   ZoneList<Expression*>* async_function_await_args =
       new (zone()) ZoneList<Expression*>(2, zone());
   Expression* generator_object =
       factory()->NewVariableProxy(generator_object_variable);
   async_function_await_args->Add(generator_object, zone());
   async_function_await_args->Add(factory()->NewVariableProxy(temp_var), zone());
 
   // The parser emits calls to AsyncFunctionAwaitCaught, but the
   // AstNumberingVisitor will rewrite this to AsyncFunctionAwaitUncaught
   // if there is no local enclosing try/catch block.
   Expression* async_function_await =
       factory()->NewCallRuntime(Context::ASYNC_FUNCTION_AWAIT_CAUGHT_INDEX,
                                 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.
-  Expression* await_assignment = factory()->NewAssignment(
-      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);
+  Expression* do_expr =
+      factory()->NewDoExpression(do_block, promise_temp_var, nopos);
 
   generator_object = factory()->NewVariableProxy(generator_object_variable);
   return factory()->NewYield(generator_object, do_expr, nopos,
                              Yield::kOnExceptionRethrow);
 }
 
 class NonPatternRewriter : public AstExpressionRewriter {
  public:
   NonPatternRewriter(uintptr_t stack_limit, Parser* parser)
       : AstExpressionRewriter(stack_limit), parser_(parser) {}
@@ skipping 1193 matching lines @@
   node->Print(Isolate::Current());
 }
 #endif  // DEBUG
 
 #undef CHECK_OK
 #undef CHECK_OK_VOID
 #undef CHECK_FAILED
 
 }  // namespace internal
 }  // namespace v8