8256474: Migrate Mutex _owner accesses to use Atomic operations

Reviewed-by: coleenp, kbarrett

src/hotspot/share/runtime/mutex.cpp

@@ -100,7 +100,7 @@ void Mutex::lock_contended(Thread* self) {
 }
 
 void Mutex::lock(Thread* self) {
-  assert(_owner != self, "invariant");
+  assert(owner() != self, "invariant");
 
   check_safepoint_state(self);
   check_rank(self);
@@ -125,7 +125,7 @@ void Mutex::lock() {
 // in the wrong way this can lead to a deadlock with the safepoint code.
 
 void Mutex::lock_without_safepoint_check(Thread * self) {
-  assert(_owner != self, "invariant");
+  assert(owner() != self, "invariant");
 
   check_no_safepoint_state(self);
   check_rank(self);
@@ -145,7 +145,7 @@ bool Mutex::try_lock_inner(bool do_rank_checks) {
   Thread * const self = Thread::current();
   // Checking the owner hides the potential difference in recursive locking behaviour
   // on some platforms.
-  if (_owner == self) {
+  if (owner() == self) {
     return false;
   }
 
@@ -308,13 +308,13 @@ Monitor::Monitor(int Rank, const char * name, bool allow_vm_block,
   Mutex(Rank, name, allow_vm_block, safepoint_check_required) {}
 
 bool Mutex::owned_by_self() const {
-  return _owner == Thread::current();
+  return owner() == Thread::current();
 }
 
 void Mutex::print_on_error(outputStream* st) const {
   st->print("[" PTR_FORMAT, p2i(this));
   st->print("] %s", _name);
-  st->print(" - owner thread: " PTR_FORMAT, p2i(_owner));
+  st->print(" - owner thread: " PTR_FORMAT, p2i(owner()));
 }
 
 // ----------------------------------------------------------------------------------
@@ -332,7 +332,7 @@ const char* print_safepoint_check(Mutex::SafepointCheckRequired safepoint_check)
 
 void Mutex::print_on(outputStream* st) const {
   st->print("Mutex: [" PTR_FORMAT "] %s - owner: " PTR_FORMAT,
-            p2i(this), _name, p2i(_owner));
+            p2i(this), _name, p2i(owner()));
   if (_allow_vm_block) {
     st->print("%s", " allow_vm_block");
   }
@@ -350,9 +350,9 @@ void Mutex::assert_owner(Thread * expected) {
   else if (expected == Thread::current()) {
     msg = "should be owned by current thread";
   }
-  assert(_owner == expected,
+  assert(owner() == expected,
          "%s: owner=" INTPTR_FORMAT ", should be=" INTPTR_FORMAT,
-         msg, p2i(_owner), p2i(expected));
+         msg, p2i(owner()), p2i(expected));
 }
 
 Mutex* Mutex::get_least_ranked_lock(Mutex* locks) {
@@ -469,8 +469,8 @@ void Mutex::set_owner_implementation(Thread *new_owner) {
     // the thread is acquiring this lock
 
     assert(new_owner == Thread::current(), "Should I be doing this?");
-    assert(_owner == NULL, "setting the owner thread of an already owned mutex");
-    _owner = new_owner; // set the owner
+    assert(owner() == NULL, "setting the owner thread of an already owned mutex");
+    raw_set_owner(new_owner); // set the owner
 
     // link "this" into the owned locks list
     this->_next = new_owner->_owned_locks;
@@ -482,14 +482,14 @@ void Mutex::set_owner_implementation(Thread *new_owner) {
   } else {
     // the thread is releasing this lock
 
-    Thread* old_owner = _owner;
+    Thread* old_owner = owner();
     _last_owner = old_owner;
     _skip_rank_check = false;
 
     assert(old_owner != NULL, "removing the owner thread of an unowned mutex");
     assert(old_owner == Thread::current(), "removing the owner thread of an unowned mutex");
 
-    _owner = NULL; // set the owner
+    raw_set_owner(NULL); // set the owner
 
     Mutex* locks = old_owner->owned_locks();
 

src/hotspot/share/runtime/mutex.hpp

@@ -26,6 +26,7 @@
 #define SHARE_RUNTIME_MUTEX_HPP
 
 #include "memory/allocation.hpp"
+#include "runtime/atomic.hpp"
 #include "runtime/os.hpp"
 
 // A Mutex/Monitor is a simple wrapper around a native lock plus condition
@@ -81,8 +82,14 @@ class Mutex : public CHeapObj<mtSynchronizer> {
        native         = max_nonleaf    +   1
   };
 
+ private:
+  // The _owner field is only set by the current thread, either to itself after it has acquired
+  // the low-level _lock, or to NULL before it has released the _lock. Accesses by any thread other
+  // than the lock owner are inherently racy.
+  Thread* volatile _owner;
+  void raw_set_owner(Thread* new_owner) { Atomic::store(&_owner, new_owner); }
+
  protected:                              // Monitor-Mutex metadata
-  Thread * volatile _owner;              // The owner of the lock
   os::PlatformMonitor _lock;             // Native monitor implementation
   char _name[MUTEX_NAME_LEN];            // Name of mutex/monitor
 
@@ -111,7 +118,7 @@ class Mutex : public CHeapObj<mtSynchronizer> {
 #endif // ASSERT
 
  protected:
-  void set_owner_implementation(Thread* owner)                        NOT_DEBUG({ _owner = owner;});
+  void set_owner_implementation(Thread* owner)                        NOT_DEBUG({ raw_set_owner(owner);});
   void check_block_state       (Thread* thread)                       NOT_DEBUG_RETURN;
   void check_safepoint_state   (Thread* thread)                       NOT_DEBUG_RETURN;
   void check_no_safepoint_state(Thread* thread)                       NOT_DEBUG_RETURN;
@@ -180,7 +187,7 @@ class Mutex : public CHeapObj<mtSynchronizer> {
   void lock(); // prints out warning if VM thread blocks
   void lock(Thread *thread); // overloaded with current thread
   void unlock();
-  bool is_locked() const                     { return _owner != NULL; }
+  bool is_locked() const                     { return owner() != NULL; }
 
   bool try_lock(); // Like lock(), but unblocking. It returns false instead
  private:
@@ -197,9 +204,9 @@ class Mutex : public CHeapObj<mtSynchronizer> {
   // A thread should not call this if failure to acquire ownership will blocks its progress
   bool try_lock_without_rank_check();
 
-  // Current owner - not not MT-safe. Can only be used to guarantee that
+  // Current owner - note not MT-safe. Can only be used to guarantee that
   // the current running thread owns the lock
-  Thread* owner() const         { return _owner; }
+  Thread* owner() const         { return Atomic::load(&_owner); }
   void set_owner(Thread* owner) { set_owner_implementation(owner); }
   bool owned_by_self() const;