commit f3740ced59abc400e718bdfa4bd8020e9516d0fa
author Martin Kustermann <kustermann@google.com> Tue Mar 02 13:11:42 2021 +0000
committer commit-bot@chromium.org <commit-bot@chromium.org> Tue Mar 02 13:11:42 2021 +0000
tree e4cf0b286c5207ca5e8d3d173088feba059aaa6c
parent af4757dd0ba5b463f554f9e6a58278a87309dc18

[vm/concurrency] Add SafepointMonitorUnlockScope to allow scoped unlocking of a monitor

In order to simplify code that needs to temporarily give up a monitor
lock, this CL adds a scoped object that releases the monitor on
construction and re-acquires it on destruction.

Issue https://github.com/dart-lang/sdk/issues/36097

TEST=Refactoring of existing code.

Change-Id: I004a04e54dcdaea009bfbef25d2a946a307e41c6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/187001
Reviewed-by: Alexander Aprelev <aam@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>

runtime/vm/lockers.cc

diff --git a/runtime/vm/lockers.cc b/runtime/vm/lockers.cc
index 30e6ef8..a39f0c6 100644
--- a/runtime/vm/lockers.cc
+++ b/runtime/vm/lockers.cc
@@ -56,8 +56,7 @@
   }
 }
 
-SafepointMonitorLocker::SafepointMonitorLocker(Monitor* monitor)
-    : monitor_(monitor) {
+void SafepointMonitorLocker::AcquireLock() {
   ASSERT(monitor_ != NULL);
   if (!monitor_->TryEnter()) {
     // We did not get the lock and could potentially block, so transition
@@ -72,6 +71,10 @@
   }
 }
 
+void SafepointMonitorLocker::ReleaseLock() {
+  monitor_->Exit();
+}
+
 Monitor::WaitResult SafepointMonitorLocker::Wait(int64_t millis) {
   Thread* thread = Thread::Current();
   if (thread != NULL) {

runtime/vm/lockers.h

diff --git a/runtime/vm/lockers.h b/runtime/vm/lockers.h
index 7213808..b4557ae 100644
--- a/runtime/vm/lockers.h
+++ b/runtime/vm/lockers.h
@@ -259,19 +259,38 @@
  */
 class SafepointMonitorLocker : public ValueObject {
  public:
-  explicit SafepointMonitorLocker(Monitor* monitor);
-  virtual ~SafepointMonitorLocker() { monitor_->Exit(); }
+  explicit SafepointMonitorLocker(Monitor* monitor) : monitor_(monitor) {
+    AcquireLock();
+  }
+  virtual ~SafepointMonitorLocker() { ReleaseLock(); }
 
   Monitor::WaitResult Wait(int64_t millis = Monitor::kNoTimeout);
 
   void NotifyAll() { monitor_->NotifyAll(); }
 
  private:
+  friend class SafepointMonitorUnlockScope;
+
+  void AcquireLock();
+  void ReleaseLock();
+
   Monitor* const monitor_;
 
   DISALLOW_COPY_AND_ASSIGN(SafepointMonitorLocker);
 };
 
+class SafepointMonitorUnlockScope : public ValueObject {
+ public:
+  explicit SafepointMonitorUnlockScope(SafepointMonitorLocker* locker)
+      : locker_(locker) {
+    locker_->ReleaseLock();
+  }
+  ~SafepointMonitorUnlockScope() { locker_->AcquireLock(); }
+
+ private:
+  SafepointMonitorLocker* locker_;
+};
+
 class RwLock {
  public:
   RwLock() {}

runtime/vm/thread_test.cc

diff --git a/runtime/vm/thread_test.cc b/runtime/vm/thread_test.cc
index 4303b5f..636aaa3 100644
--- a/runtime/vm/thread_test.cc
+++ b/runtime/vm/thread_test.cc
@@ -1102,4 +1102,23 @@
   EXPECT(state.elapsed_us > 2 * 500 * 1000);
 }
 
+ISOLATE_UNIT_TEST_CASE(SafepointMonitorUnlockScope) {
+  // This test uses ASSERT instead of EXPECT because IsOwnedByCurrentThread is
+  // only available in debug mode. Since our vm/cc tests run in DEBUG mode that
+  // is sufficent for this test.
+  Monitor monitor;
+  {
+    SafepointMonitorLocker ml(&monitor);
+    ASSERT(monitor.IsOwnedByCurrentThread());
+    {
+      SafepointMonitorUnlockScope ml_unlocker(&ml);
+      ASSERT(!monitor.IsOwnedByCurrentThread());
+      {
+        SafepointMonitorLocker inner_ml(&monitor);
+        ASSERT(monitor.IsOwnedByCurrentThread());
+      }
+    }
+  }
+}
+
 }  // namespace dart