- Add assertions in MutexLocker/MonitorLocker to ensure that the code enclosed

runtime/vm/lockers.h

Index: runtime/vm/lockers.h
diff --git a/runtime/vm/lockers.h b/runtime/vm/lockers.h
index fd324f44811551673dd7a25f823abac28bac1b2e..a0b1a179be8f0aacce55c0d53b35777e442ee7b6 100644
--- a/runtime/vm/lockers.h
+++ b/runtime/vm/lockers.h
@@ -13,37 +13,156 @@
 
 namespace dart {
 
+const bool kNoSafepointScope = true;
+const bool kDontAssertNoSafepointScope = false;
+
+/*
+ * Normal mutex locker :
+ * This locker abstraction should only be used when the enclosing code can
+ * not trigger a safepoint. In debug mode this class increments the
+ * no_safepoint_scope_depth variable for the current thread when the lock is
+ * taken and decrements it when the lock is released. NOTE: please do not use
+ * the passed in mutex object independent of the locker class, For example the
+ * code below will not assert correctly:
+ *    {
+ *      MutexLocker ml(m);
+ *      ....
+ *      m->Exit();
+ *      ....
+ *      m->Enter();
+ *      ...
+ *    }
+ * Always use the locker object even when the lock needs to be released
+ * temporarily, e.g:
+ *    {
+ *      MutexLocker ml(m);
+ *      ....
+ *      ml.Exit();
+ *      ....
+ *      ml.Enter();
+ *      ...
+ *    }
+ */
 class MutexLocker : public ValueObject {
  public:
-  explicit MutexLocker(Mutex* mutex) : mutex_(mutex) {
+  explicit MutexLocker(Mutex* mutex, bool no_safepoint_scope = true)
+      : mutex_(mutex), no_safepoint_scope_(no_safepoint_scope) {
     ASSERT(mutex != NULL);
-    // TODO(iposva): Consider adding a no GC scope here.
+#if defined(DEBUG)
+    if (no_safepoint_scope_) {
+      Thread* thread = Thread::Current();
+      if (thread != NULL) {
+        thread->IncrementNoSafepointScopeDepth();
+      } else {
+        no_safepoint_scope_ = false;
+      }
+    }
+#endif
     mutex_->Lock();
   }
 
   virtual ~MutexLocker() {
     mutex_->Unlock();
-    // TODO(iposva): Consider decrementing the no GC scope here.
+#if defined(DEBUG)
+    if (no_safepoint_scope_) {
+      Thread::Current()->DecrementNoSafepointScopeDepth();
+    }
+#endif
+  }
+
+  void Lock() const {
+#if defined(DEBUG)
+    if (no_safepoint_scope_) {
+      Thread::Current()->IncrementNoSafepointScopeDepth();
+    }
+#endif
+    mutex_->Lock();
+  }
+  void Unlock() const {
+    mutex_->Unlock();
+#if defined(DEBUG)
+    if (no_safepoint_scope_) {
+      Thread::Current()->DecrementNoSafepointScopeDepth();
+    }
+#endif
   }
 
  private:
   Mutex* const mutex_;
+  bool no_safepoint_scope_;
 
   DISALLOW_COPY_AND_ASSIGN(MutexLocker);
 };
 
-
+/*
+ * Normal monitor locker :
+ * This locker abstraction should only be used when the enclosing code can
+ * not trigger a safepoint. In debug mode this class increments the
+ * no_safepoint_scope_depth variable for the current thread when the lock is
+ * taken and decrements it when the lock is released. NOTE: please do not use
+ * the passed in mutex object independent of the locker class, For example the
+ * code below will not assert correctly:
+ *    {
+ *      MonitorLocker ml(m);
+ *      ....
+ *      m->Exit();
+ *      ....
+ *      m->Enter();
+ *      ...
+ *    }
+ * Always use the locker object even when the lock needs to be released
+ * temporarily, e.g:
+ *    {
+ *      MonitorLocker ml(m);
+ *      ....
+ *      ml.Exit();
+ *      ....
+ *      ml.Enter();
+ *      ...
+ *    }
+ */
 class MonitorLocker : public ValueObject {
  public:
-  explicit MonitorLocker(Monitor* monitor) : monitor_(monitor) {
+  explicit MonitorLocker(Monitor* monitor, bool no_safepoint_scope = true)
+      : monitor_(monitor), no_safepoint_scope_(no_safepoint_scope) {
     ASSERT(monitor != NULL);
-    // TODO(iposva): Consider adding a no GC scope here.
+#if defined(DEBUG)
+    if (no_safepoint_scope_) {
+      Thread* thread = Thread::Current();
+      if (thread != NULL) {
+        thread->IncrementNoSafepointScopeDepth();
+      } else {
+        no_safepoint_scope_ = false;
+      }
+    }
+#endif
     monitor_->Enter();
   }
 
   virtual ~MonitorLocker() {
     monitor_->Exit();
-    // TODO(iposva): Consider decrementing the no GC scope here.
+#if defined(DEBUG)
+    if (no_safepoint_scope_) {
+      Thread::Current()->DecrementNoSafepointScopeDepth();
+    }
+#endif
+  }
+
+  void Enter() const {
+#if defined(DEBUG)
+    if (no_safepoint_scope_) {
+      Thread::Current()->IncrementNoSafepointScopeDepth();
+    }
+#endif
+    monitor_->Enter();
+  }
+  void Exit() const {
+    monitor_->Exit();
+#if defined(DEBUG)
+    if (no_safepoint_scope_) {
+      Thread::Current()->DecrementNoSafepointScopeDepth();
+    }
+#endif
   }
 
   Monitor::WaitResult Wait(int64_t millis = Monitor::kNoTimeout) {
@@ -67,16 +186,29 @@ class MonitorLocker : public ValueObject {
 
  private:
   Monitor* const monitor_;
+  bool no_safepoint_scope_;
 
   DISALLOW_COPY_AND_ASSIGN(MonitorLocker);
 };
 
-
-// SafepointMutexLocker objects are used in code where the locks are
-// more coarse grained and a safepoint operation could be potentially
-// triggered while holding this lock. This ensures that other threads
-// which try to acquire the same lock will be marked as being at a
-// safepoint when they are blocked.
+/*
+ * Safepoint mutex locker :
+ * This locker abstraction should be used when the enclosing code could
+ * potentially trigger a safepoint.
+ * This locker ensures that other threads that try to acquire the same lock
+ * will be marked as being at a safepoint if they get blocked trying to
+ * acquire the lock.
+ * NOTE: please do not use the passed in mutex object independent of the locker
+ * class, For example the code below will not work correctly:
+ *    {
+ *      SafepointMutexLocker ml(m);
+ *      ....
+ *      m->Exit();
+ *      ....
+ *      m->Enter();
+ *      ...
+ *    }
+ */
 class SafepointMutexLocker : public ValueObject {
  public:
   explicit SafepointMutexLocker(Mutex* mutex);
@@ -90,6 +222,39 @@ class SafepointMutexLocker : public ValueObject {
   DISALLOW_COPY_AND_ASSIGN(SafepointMutexLocker);
 };
 
+/*
+ * Safepoint monitor locker :
+ * This locker abstraction should be used when the enclosing code could
+ * potentially trigger a safepoint.
+ * This locker ensures that other threads that try to acquire the same lock
+ * will be marked as being at a safepoint if they get blocked trying to
+ * acquire the lock.
+ * NOTE: please do not use the passed in monitor object independent of the
+ * locker class, For example the code below will not work correctly:
+ *    {
+ *      SafepointMonitorLocker ml(m);
+ *      ....
+ *      m->Exit();
+ *      ....
+ *      m->Enter();
+ *      ...
+ *    }
+ */
+class SafepointMonitorLocker : public ValueObject {
+ public:
+  explicit SafepointMonitorLocker(Monitor* monitor);
+  virtual ~SafepointMonitorLocker() {
+    monitor_->Exit();
+  }
+
+  Monitor::WaitResult Wait(int64_t millis = Monitor::kNoTimeout);
+
+ private:
+  Monitor* const monitor_;
+
+  DISALLOW_COPY_AND_ASSIGN(SafepointMonitorLocker);
+};
+
 }  // namespace dart