Version 1.17.1

An earlier commit was also named 1.17.1, but that was an error. That earlier commit was released as 1.17.0. Cherry-pick 'f4029bf3ee72c6667f4099a0e2e747b22fe45c32' to stable Cherry-pick 'e803457081ee5c0efcd05d924c168ceb4dc534c7' to stable Cherry-pick '97a8c4caffd2e758c60a3e4502f07d6886ee0363' to stable Cherry-pick '9d5e1331149062cd5de4ed5a54841c03872f8edd' to stable

Author: whesse

CHANGELOG.md

@@ -1,4 +1,14 @@
-## 1.17.0 - 2016-06-06
+## 1.17.1 - 2016-06-10
+
+Patch release, resolves two issues:
+
+* VM: Fixes a bug that caused crashes in async functions.
+(SDK issue [26668](https://github.com/dart-lang/sdk/issues/26668))
+
+* VM: Fixes a bug that caused garbage collection of reachable weak properties.
+(https://codereview.chromium.org/2041413005)
+
+## 1.17.0 - 2016-06-08
 
 ### Core library changes
 * `dart:convert`

runtime/platform/assert.h

@@ -281,6 +281,12 @@ T DynamicAssertionHelper::NotNull(const T p) {
 #endif // if defined(DEBUG)
 
 
+#define RELEASE_ASSERT(cond) \
+ do { \
+ if (!(cond)) dart::Assert(__FILE__, __LINE__).Fail("expected: %s", #cond); \
+ } while (false)
+
+
 // The COMPILE_ASSERT macro can be used to verify that a compile time
 // expression is true. For example, you could use it to verify the
 // size of a static array:

runtime/vm/gc_marker.cc

@@ -177,12 +177,43 @@ class MarkingVisitorBase : public ObjectPointerVisitor {
  return class_stats_size_[class_id];
  }
 
- // Returns true if some non-zero amount of work was performed.
- bool DrainMarkingStack() {
+ bool ProcessPendingWeakProperties() {
+ bool marked = false;
+ RawWeakProperty* cur_weak = delayed_weak_properties_;
+ delayed_weak_properties_ = NULL;
+ while (cur_weak != NULL) {
+ uword next_weak = cur_weak->ptr()->next_;
+ RawObject* raw_key = cur_weak->ptr()->key_;
+ // Reset the next pointer in the weak property.
+ cur_weak->ptr()->next_ = 0;
+ if (raw_key->IsMarked()) {
+ RawObject* raw_val = cur_weak->ptr()->value_;
+ marked = marked || (raw_val->IsHeapObject() && !raw_val->IsMarked());
+
+ // The key is marked so we make sure to properly visit all pointers
+ // originating from this weak property.
+ VisitingOldObject(cur_weak);
+ cur_weak->VisitPointers(this);
+ } else {
+ // Requeue this weak property to be handled later.
+ EnqueueWeakProperty(cur_weak);
+ }
+ // Advance to next weak property in the queue.
+ cur_weak = reinterpret_cast<RawWeakProperty*>(next_weak);
+ }
+ VisitingOldObject(NULL);
+ return marked;
+ }
+
+ void DrainMarkingStack() {
  RawObject* raw_obj = work_list_.Pop();
+ if ((raw_obj == NULL) && ProcessPendingWeakProperties()) {
+ raw_obj = work_list_.Pop();
+ }
+
  if (raw_obj == NULL) {
  ASSERT(visiting_old_object_ == NULL);
- return false;
+ return;
  }
  do {
  do {
@@ -200,33 +231,13 @@ class MarkingVisitorBase : public ObjectPointerVisitor {
  } while (raw_obj != NULL);
 
  // Marking stack is empty.
- // Process all the pending weak properties in this visitor.
- RawWeakProperty* cur_weak = delayed_weak_properties_;
- delayed_weak_properties_ = NULL;
- while (cur_weak != NULL) {
- uword next_weak = cur_weak->ptr()->next_;
- RawObject* raw_key = cur_weak->ptr()->key_;
- // Reset the next pointer in the weak property.
- cur_weak->ptr()->next_ = 0;
- if (raw_key->IsMarked()) {
- // The key is marked so we make sure to properly visit all pointers
- // originating from this weak property.
- VisitingOldObject(cur_weak);
- cur_weak->VisitPointers(this);
- } else {
- // Requeue this weak property to be handled later.
- EnqueueWeakProperty(cur_weak);
- }
- // Advance to next weak property in the queue.
- cur_weak = reinterpret_cast<RawWeakProperty*>(next_weak);
- }
+ ProcessPendingWeakProperties();
 
  // Check whether any further work was pushed either by other markers or
  // by the handling of weak properties.
  raw_obj = work_list_.Pop();
  } while (raw_obj != NULL);
  VisitingOldObject(NULL);
- return true;
  }
 
  void VisitPointers(RawObject** first, RawObject** last) {
@@ -282,6 +293,7 @@ class MarkingVisitorBase : public ObjectPointerVisitor {
  while (cur_weak != NULL) {
  uword next_weak = cur_weak->ptr()->next_;
  cur_weak->ptr()->next_ = 0;
+ RELEASE_ASSERT(!cur_weak->ptr()->key_->IsMarked());
  WeakProperty::Clear(cur_weak);
  weak_properties_cleared++;
  // Advance to next weak property in the queue.
@@ -569,28 +581,54 @@ class MarkTask : public ThreadPool::Task {
  skipped_code_functions);
  // Phase 1: Iterate over roots and drain marking stack in tasks.
  marker_->IterateRoots(isolate_, &visitor, task_index_, num_tasks_);
- do {
- visitor.DrainMarkingStack();
 
- // I can't find more work right now. If no other task is busy,
- // then there will never be more work (NB: 1 is *before* decrement).
- if (AtomicOperations::FetchAndDecrement(num_busy_) == 1) break;
-
- // Wait for some work to appear.
- // TODO(iposva): Replace busy-waiting with a solution using Monitor,
- // and redraw the boundaries between stack/visitor/task as needed.
- while (marking_stack_->IsEmpty() &&
- AtomicOperations::LoadRelaxed(num_busy_) > 0) {
+ bool more_to_mark = false;
+ do {
+ do {
+ visitor.DrainMarkingStack();
+
+ // I can't find more work right now. If no other task is busy,
+ // then there will never be more work (NB: 1 is *before* decrement).
+ if (AtomicOperations::FetchAndDecrement(num_busy_) == 1) break;
+
+ // Wait for some work to appear.
+ // TODO(iposva): Replace busy-waiting with a solution using Monitor,
+ // and redraw the boundaries between stack/visitor/task as needed.
+ while (marking_stack_->IsEmpty() &&
+ AtomicOperations::LoadRelaxed(num_busy_) > 0) {
+ }
+
+ // If no tasks are busy, there will never be more work.
+ if (AtomicOperations::LoadRelaxed(num_busy_) == 0) break;
+
+ // I saw some work; get busy and compete for it.
+ AtomicOperations::FetchAndIncrement(num_busy_);
+ } while (true);
+ // Wait for all markers to stop.
+ barrier_->Sync();
+ ASSERT(AtomicOperations::LoadRelaxed(num_busy_) == 0);
+
+ // Check if we have any pending properties with marked keys.
+ // Those might have been marked by another marker.
+ more_to_mark = visitor.ProcessPendingWeakProperties();
+ if (more_to_mark) {
+ // We have more work to do. Notify others.
+ AtomicOperations::FetchAndIncrement(num_busy_);
  }
 
- // If no tasks are busy, there will never be more work.
- if (AtomicOperations::LoadRelaxed(num_busy_) == 0) break;
-
- // I saw some work; get busy and compete for it.
- AtomicOperations::FetchAndIncrement(num_busy_);
- } while (true);
- ASSERT(AtomicOperations::LoadRelaxed(num_busy_) == 0);
- barrier_->Sync();
+ // Wait for all other markers to finish processing their pending
+ // weak properties and decide if they need to continue marking.
+ // Caveat: we need two barriers here to make this decision in lock step
+ // between all markers and the main thread.
+ barrier_->Sync();
+ if (!more_to_mark && (AtomicOperations::LoadRelaxed(num_busy_) > 0)) {
+ // All markers continue to marker as long as any single marker has
+ // some work to do.
+ AtomicOperations::FetchAndIncrement(num_busy_);
+ more_to_mark = true;
+ }
+ barrier_->Sync();
+ } while (more_to_mark);
 
  // Phase 2: Weak processing and follow-up marking on main thread.
  barrier_->Sync();
@@ -688,7 +726,19 @@ void GCMarker::MarkObjects(Isolate* isolate,
  ThreadPool* pool = Dart::thread_pool();
  pool->Run(mark_task);
  }
- barrier.Sync();
+ bool more_to_mark = false;
+ do {
+ // Wait for all markers to stop.
+ barrier.Sync();
+
+ // Wait for all markers to go through weak properties and verify
+ // that there are no more objects to mark.
+ // Note: we need to have two barriers here because we want all markers
+ // and main thread to make decisions in lock step.
+ barrier.Sync();
+ more_to_mark = AtomicOperations::LoadRelaxed(&num_busy) > 0;
+ barrier.Sync();
+ } while (more_to_mark);
 
  // Phase 2: Weak processing on main thread.
  {

runtime/vm/parser.cc

@@ -10057,25 +10057,28 @@ AstNode* Parser::ParseStatement() {
  ConsumeToken();
  if (CurrentToken() != Token::kSEMICOLON) {
  const TokenPosition expr_pos = TokenPos();
+ const int function_level = current_block_->scope->function_level();
  if (current_function().IsGenerativeConstructor() &&
- (current_block_->scope->function_level() == 0)) {
+ (function_level == 0)) {
  ReportError(expr_pos,
  "return of a value is not allowed in constructors");
  } else if (current_function().IsGeneratorClosure() &&
- (current_block_->scope->function_level() == 0)) {
+ (function_level == 0)) {
  ReportError(expr_pos, "generator functions may not return a value");
  }
  AstNode* expr = ParseAwaitableExpr(kAllowConst, kConsumeCascades, NULL);
  if (I->type_checks() &&
- current_function().IsAsyncClosure() &&
- (current_block_->scope->function_level() == 0)) {
+ (((function_level == 0) && current_function().IsAsyncClosure()) ||
+  ((function_level > 0) && current_function().IsAsyncFunction()))) {
  // In checked mode, when the declared result type is Future<T>, verify
  // that the returned expression is of type T or Future<T> as follows:
  // return temp = expr, temp is Future ? temp as Future<T> : temp as T;
  // In case of a mismatch, we need a TypeError and not a CastError, so
  // we do not actually implement an "as" test, but an "assignable" test.
- const Function& async_func =
- Function::Handle(Z, current_function().parent_function());
+ Function& async_func = Function::Handle(Z, current_function().raw());
+ if (function_level == 0) {
+ async_func = async_func.parent_function();
+ }
  const AbstractType& result_type =
  AbstractType::ZoneHandle(Z, async_func.result_type());
  const Class& future_class =
@@ -10087,32 +10090,41 @@ AstNode* Parser::ParseStatement() {
  if (!result_type_args.IsNull() && (result_type_args.Length() == 1)) {
  const AbstractType& result_type_arg =
  AbstractType::ZoneHandle(Z, result_type_args.TypeAt(0));
- LetNode* checked_expr = new(Z) LetNode(expr_pos);
- LocalVariable* temp = checked_expr->AddInitializer(expr);
- temp->set_is_final();
- const AbstractType& future_type =
- AbstractType::ZoneHandle(Z, future_class.RareType());
- AstNode* is_future = new(Z) LoadLocalNode(expr_pos, temp);
- is_future = new(Z) ComparisonNode(expr_pos,
- Token::kIS,
- is_future,
- new(Z) TypeNode(expr_pos,
- future_type));
- AstNode* as_future_t = new(Z) LoadLocalNode(expr_pos, temp);
- as_future_t = new(Z) AssignableNode(expr_pos,
- as_future_t,
- result_type,
- Symbols::FunctionResult());
- AstNode* as_t = new(Z) LoadLocalNode(expr_pos, temp);
- as_t = new(Z) AssignableNode(expr_pos,
- as_t,
- result_type_arg,
- Symbols::FunctionResult());
- checked_expr->AddNode(new(Z) ConditionalExprNode(expr_pos,
- is_future,
- as_future_t,
- as_t));
- expr = checked_expr;
+ if (function_level == 0) {
+ // Parsing and generating code for async closure.
+ LetNode* checked_expr = new(Z) LetNode(expr_pos);
+ LocalVariable* temp = checked_expr->AddInitializer(expr);
+ temp->set_is_final();
+ const AbstractType& future_type =
+ AbstractType::ZoneHandle(Z, future_class.RareType());
+ AstNode* is_future = new(Z) LoadLocalNode(expr_pos, temp);
+ is_future = new(Z) ComparisonNode(expr_pos,
+ Token::kIS,
+ is_future,
+ new(Z) TypeNode(expr_pos,
+ future_type));
+ AstNode* as_future_t = new(Z) LoadLocalNode(expr_pos, temp);
+ as_future_t = new(Z) AssignableNode(expr_pos,
+ as_future_t,
+ result_type,
+ Symbols::FunctionResult());
+ AstNode* as_t = new(Z) LoadLocalNode(expr_pos, temp);
+ as_t = new(Z) AssignableNode(expr_pos,
+ as_t,
+ result_type_arg,
+ Symbols::FunctionResult());
+ checked_expr->AddNode(new(Z) ConditionalExprNode(expr_pos,
+ is_future,
+ as_future_t,
+ as_t));
+ expr = checked_expr;
+ } else {
+ // Parsing async function, but not generating async closure code.
+ if (!result_type_arg.IsInstantiated()) {
+ // Make sure that the instantiator is captured.
+ CaptureInstantiator();
+ }
+ }
  }
  }
  }

tests/language/language_analyzer2.status

@@ -4,6 +4,8 @@
 
 [ $compiler == dart2analyzer ]
 
+regress_26668_test: Fail # Issue 26678
+
 # Runtime negative test. No static errors or warnings.
 closure_call_wrong_argument_count_negative_test: skip
 

tests/language/regress_26668_test.dart

@@ -0,0 +1,18 @@
+// Copyright (c) 2016, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+// Tests that the VM does not crash on weird corner cases of class Math.
+
+import 'dart:async';
+
+main() async {
+ var myClass = new CustomClass<int>();
+ await myClass.processData();
+}
+
+class CustomClass<T> {
+ Future<T> processData() async {
+ return 0;
+ }
+}
+

tools/VERSION

@@ -26,6 +26,6 @@
 CHANNEL stable
 MAJOR 1
 MINOR 17
-PATCH 0
+PATCH 1
 PRERELEASE 0
 PRERELEASE_PATCH 0