Allow for recursive invocation of SafepointOperationScopes.

R=fschneider@google.com

Review URL: https://codereview.chromium.org/2126413002 .

runtime/vm/isolate.cc

@@ -2527,7 +2527,7 @@ Thread* Isolate::ScheduleThread(bool is_mutator, bool bypass_safepoint) {
 
     // If a safepoint operation is in progress wait for it
     // to finish before scheduling this thread in.
-    while (!bypass_safepoint && safepoint_handler()->safepoint_in_progress()) {
+    while (!bypass_safepoint && safepoint_handler()->SafepointInProgress()) {
       ml.Wait();
     }
 

runtime/vm/safepoint.cc

@@ -41,12 +41,14 @@ SafepointHandler::SafepointHandler(Isolate* isolate)
     : isolate_(isolate),
       safepoint_lock_(new Monitor()),
       number_threads_not_at_safepoint_(0),
-      safepoint_in_progress_(false) {
+      safepoint_operation_count_(0),
+      owner_(NULL) {
 }
 
 
 SafepointHandler::~SafepointHandler() {
-  ASSERT(safepoint_in_progress_ == false);
+  ASSERT(owner_ == NULL);
+  ASSERT(safepoint_operation_count_ == 0);
   delete safepoint_lock_;
   safepoint_lock_ = NULL;
   isolate_ = NULL;
@@ -63,12 +65,19 @@ void SafepointHandler::SafepointThreads(Thread* T) {
 
     // Now check to see if a safepoint operation is already in progress
     // for this isolate, block if an operation is in progress.
-    while (safepoint_in_progress()) {
+    while (SafepointInProgress()) {
+      // If we are recursively invoking a Safepoint operation then we
+      // just increment the count and return, otherwise we wait for the
+      // safepoint operation to be done.
+      if (owner_ == T) {
+        increment_safepoint_operation_count();
+        return;
+      }
       sl.WaitWithSafepointCheck(T);
     }
 
-    // Set safepoint in progress by this thread.
-    set_safepoint_in_progress(true);
+    // Set safepoint in progress state by this thread.
+    SetSafepointInProgress(T);
 
     // Go over the active thread list and ensure that all threads active
     // in the isolate reach a safepoint.
@@ -114,6 +123,14 @@ void SafepointHandler::ResumeThreads(Thread* T) {
   // First resume all the threads which are blocked for the safepoint
   // operation.
   MonitorLocker sl(threads_lock());
+
+  // First check if we are in a recursive safepoint operation, in that case
+  // we just decrement safepoint_operation_count and return.
+  ASSERT(SafepointInProgress());
+  if (safepoint_operation_count() > 1) {
+    decrement_safepoint_operation_count();
+    return;
+  }
   Thread* current = isolate()->thread_registry()->active_list();
   while (current != NULL) {
     MonitorLocker tl(current->thread_lock());
@@ -127,10 +144,10 @@ void SafepointHandler::ResumeThreads(Thread* T) {
     }
     current = current->next();
   }
-  // Now set the safepoint_in_progress_ flag to false and notify all threads
+  // Now reset the safepoint_in_progress_ state and notify all threads
   // that are waiting to enter the isolate or waiting to start another
   // safepoint operation.
-  set_safepoint_in_progress(false);
+  ResetSafepointInProgress(T);
   sl.NotifyAll();
 }
 

runtime/vm/safepoint.h

@@ -33,21 +33,45 @@ class SafepointHandler {
   void EnterSafepointUsingLock(Thread* T);
   void ExitSafepointUsingLock(Thread* T);
 
-  void SafepointThreads(Thread* T);
-  void ResumeThreads(Thread* T);
-
   void BlockForSafepoint(Thread* T);
 
  private:
+  void SafepointThreads(Thread* T);
+  void ResumeThreads(Thread* T);
+
   Isolate* isolate() const { return isolate_; }
   Monitor* threads_lock() const { return isolate_->threads_lock(); }
-  bool safepoint_in_progress() const {
+  bool SafepointInProgress() const {
     ASSERT(threads_lock()->IsOwnedByCurrentThread());
-    return safepoint_in_progress_;
+    return ((safepoint_operation_count_ > 0) && (owner_ != NULL));
   }
-  void set_safepoint_in_progress(bool value) {
+  void SetSafepointInProgress(Thread* T) {
     ASSERT(threads_lock()->IsOwnedByCurrentThread());
-    safepoint_in_progress_ = value;
+    ASSERT(owner_ == NULL);
+    ASSERT(safepoint_operation_count_ == 0);
+    safepoint_operation_count_ = 1;
+    owner_ = T;
+  }
+  void ResetSafepointInProgress(Thread* T) {
+    ASSERT(threads_lock()->IsOwnedByCurrentThread());
+    ASSERT(owner_ == T);
+    ASSERT(safepoint_operation_count_ == 1);
+    safepoint_operation_count_ = 0;
+    owner_ = NULL;
+  }
+  int32_t safepoint_operation_count() const {
+    ASSERT(threads_lock()->IsOwnedByCurrentThread());
+    return safepoint_operation_count_;
+  }
+  void increment_safepoint_operation_count() {
+    ASSERT(threads_lock()->IsOwnedByCurrentThread());
+    ASSERT(safepoint_operation_count_ < kMaxInt32);
+    safepoint_operation_count_ += 1;
+  }
+  void decrement_safepoint_operation_count() {
+    ASSERT(threads_lock()->IsOwnedByCurrentThread());
+    ASSERT(safepoint_operation_count_ > 0);
+    safepoint_operation_count_ -= 1;
   }
 
   Isolate* isolate_;
@@ -57,8 +81,14 @@ class SafepointHandler {
   Monitor* safepoint_lock_;
   int32_t number_threads_not_at_safepoint_;
 
-  // Flag to indicate if a safepoint operation is currently in progress.
-  bool safepoint_in_progress_;
+  // Count that indicates if a safepoint operation is currently in progress
+  // and also tracks the number of recursive safepoint operations on the
+  // same thread.
+  int32_t safepoint_operation_count_;
+
+  // If a safepoint operation is currently in progress, this field contains
+  // the thread that initiated the safepoint operation, otherwise it is NULL.
+  Thread* owner_;
 
   friend class Isolate;
   friend class SafepointOperationScope;

runtime/vm/thread_test.cc

@@ -405,6 +405,25 @@ VM_TEST_CASE(SafepointTestVM) {
 }
 
 
+// Test case for recursive safepoint operations.
+VM_TEST_CASE(RecursiveSafepointTest1) {
+  intptr_t count = 0;
+  {
+    SafepointOperationScope safepoint_scope(thread);
+    count += 1;
+    {
+      SafepointOperationScope safepoint_scope(thread);
+      count += 1;
+      {
+        SafepointOperationScope safepoint_scope(thread);
+        count += 1;
+      }
+    }
+  }
+  EXPECT(count == 3);
+}
+
+
 VM_TEST_CASE(ThreadIterator_Count) {
   intptr_t thread_count_0 = 0;
   intptr_t thread_count_1 = 0;
@@ -525,6 +544,46 @@ VM_TEST_CASE(SafepointTestVM2) {
 }
 
 
+// Test recursive safepoint operation scopes with other threads trying
+// to also start a safepoint operation scope.
+VM_TEST_CASE(RecursiveSafepointTest2) {
+  Isolate* isolate = thread->isolate();
+  Monitor monitor;
+  intptr_t expected_count = 0;
+  intptr_t total_done = 0;
+  intptr_t exited = 0;
+  for (int i = 0; i < SafepointTestTask::kTaskCount; i++) {
+    Dart::thread_pool()->Run(new SafepointTestTask(
+        isolate, &monitor, &expected_count, &total_done, &exited));
+  }
+  bool all_helpers = false;
+  do {
+    SafepointOperationScope safepoint_scope(thread);
+    {
+      SafepointOperationScope safepoint_scope(thread);
+      MonitorLocker ml(&monitor);
+      if (expected_count == SafepointTestTask::kTaskCount) {
+        all_helpers = true;
+      }
+    }
+  } while (!all_helpers);
+  String& label = String::Handle(String::New("foo"));
+  UserTag& tag = UserTag::Handle(UserTag::New(label));
+  isolate->set_current_tag(tag);
+  bool all_exited = false;
+  do {
+    SafepointOperationScope safepoint_scope(thread);
+    {
+      SafepointOperationScope safepoint_scope(thread);
+      MonitorLocker ml(&monitor);
+      if (exited == SafepointTestTask::kTaskCount) {
+        all_exited = true;
+      }
+    }
+  } while (!all_exited);
+}
+
+
 class AllocAndGCTask : public ThreadPool::Task {
  public:
   AllocAndGCTask(Isolate* isolate,