8218975: Bug in macOSX kernel's pthread support
authorkbarrett
Tue, 19 Mar 2019 14:32:41 -0400
changeset 54195 4b6a629d0615
parent 54194 c2238a12f259
child 54196 6146ab937899
8218975: Bug in macOSX kernel's pthread support Summary: Use freelist of pthread_mutex/condvar pairs on macOSX. Reviewed-by: tschatzl, dholmes, dcubed Contributed-by: kim.barrett@oracle.com, patricio.chilano.mateo@oracle.com
src/hotspot/os/posix/os_posix.cpp
src/hotspot/os/posix/os_posix.hpp
src/hotspot/os/posix/os_posix.inline.hpp
--- a/src/hotspot/os/posix/os_posix.cpp	Tue Mar 19 14:31:52 2019 -0400
+++ b/src/hotspot/os/posix/os_posix.cpp	Tue Mar 19 14:32:41 2019 -0400
@@ -1671,6 +1671,8 @@
   if ((status = pthread_mutexattr_settype(_mutexAttr, PTHREAD_MUTEX_NORMAL)) != 0) {
     fatal("pthread_mutexattr_settype: %s", os::strerror(status));
   }
+  // Solaris has it's own PlatformMonitor, distinct from the one for POSIX.
+  NOT_SOLARIS(os::PlatformMonitor::init();)
 }
 
 #ifndef SOLARIS
@@ -2250,20 +2252,64 @@
 
 // Platform Monitor implementation
 
-os::PlatformMonitor::PlatformMonitor() {
+os::PlatformMonitor::Impl::Impl() : _next(NULL) {
   int status = pthread_cond_init(&_cond, _condAttr);
   assert_status(status == 0, status, "cond_init");
   status = pthread_mutex_init(&_mutex, _mutexAttr);
   assert_status(status == 0, status, "mutex_init");
 }
 
-os::PlatformMonitor::~PlatformMonitor() {
+os::PlatformMonitor::Impl::~Impl() {
   int status = pthread_cond_destroy(&_cond);
   assert_status(status == 0, status, "cond_destroy");
   status = pthread_mutex_destroy(&_mutex);
   assert_status(status == 0, status, "mutex_destroy");
 }
 
+#if PLATFORM_MONITOR_IMPL_INDIRECT
+
+pthread_mutex_t os::PlatformMonitor::_freelist_lock;
+os::PlatformMonitor::Impl* os::PlatformMonitor::_freelist = NULL;
+
+void os::PlatformMonitor::init() {
+  int status = pthread_mutex_init(&_freelist_lock, _mutexAttr);
+  assert_status(status == 0, status, "freelist lock init");
+}
+
+struct os::PlatformMonitor::WithFreeListLocked : public StackObj {
+  WithFreeListLocked() {
+    int status = pthread_mutex_lock(&_freelist_lock);
+    assert_status(status == 0, status, "freelist lock");
+  }
+
+  ~WithFreeListLocked() {
+    int status = pthread_mutex_unlock(&_freelist_lock);
+    assert_status(status == 0, status, "freelist unlock");
+  }
+};
+
+os::PlatformMonitor::PlatformMonitor() {
+  {
+    WithFreeListLocked wfl;
+    _impl = _freelist;
+    if (_impl != NULL) {
+      _freelist = _impl->_next;
+      _impl->_next = NULL;
+      return;
+    }
+  }
+  _impl = new Impl();
+}
+
+os::PlatformMonitor::~PlatformMonitor() {
+  WithFreeListLocked wfl;
+  assert(_impl->_next == NULL, "invariant");
+  _impl->_next = _freelist;
+  _freelist = _impl;
+}
+
+#endif // PLATFORM_MONITOR_IMPL_INDIRECT
+
 // Must already be locked
 int os::PlatformMonitor::wait(jlong millis) {
   assert(millis >= 0, "negative timeout");
@@ -2278,7 +2324,7 @@
     to_abstime(&abst, millis * (NANOUNITS / MILLIUNITS), false, false);
 
     int ret = OS_TIMEOUT;
-    int status = pthread_cond_timedwait(&_cond, &_mutex, &abst);
+    int status = pthread_cond_timedwait(cond(), mutex(), &abst);
     assert_status(status == 0 || status == ETIMEDOUT,
                   status, "cond_timedwait");
     if (status == 0) {
@@ -2286,7 +2332,7 @@
     }
     return ret;
   } else {
-    int status = pthread_cond_wait(&_cond, &_mutex);
+    int status = pthread_cond_wait(cond(), mutex());
     assert_status(status == 0, status, "cond_wait");
     return OS_OK;
   }
--- a/src/hotspot/os/posix/os_posix.hpp	Tue Mar 19 14:31:52 2019 -0400
+++ b/src/hotspot/os/posix/os_posix.hpp	Tue Mar 19 14:32:41 2019 -0400
@@ -232,15 +232,60 @@
   PlatformParker();
 };
 
+// Workaround for a bug in macOSX kernel's pthread support (fixed in Mojave?).
+// Avoid ever allocating a pthread_mutex_t at the same address as one of our
+// former pthread_cond_t, by using a freelist of mutex/condvar pairs.
+// Conditional to avoid extra indirection and padding loss on other platforms.
+#ifdef __APPLE__
+#define PLATFORM_MONITOR_IMPL_INDIRECT 1
+#else
+#define PLATFORM_MONITOR_IMPL_INDIRECT 0
+#endif
+
 // Platform specific implementation that underpins VM Monitor/Mutex class
 class PlatformMonitor : public CHeapObj<mtSynchronizer> {
- private:
-  pthread_mutex_t _mutex; // Native mutex for locking
-  pthread_cond_t  _cond;  // Native condition variable for blocking
+  class Impl : public CHeapObj<mtSynchronizer> {
+  public:
+    pthread_mutex_t _mutex;
+    pthread_cond_t _cond;
+    Impl* _next;
+
+    Impl();
+    ~Impl();
+  };
+
+#if PLATFORM_MONITOR_IMPL_INDIRECT
+
+  Impl* _impl;
+
+  pthread_mutex_t* mutex() { return &(_impl->_mutex); }
+  pthread_cond_t* cond() { return &(_impl->_cond); }
+
+  class WithFreeListLocked;
+  static pthread_mutex_t _freelist_lock;
+  static Impl* _freelist;
 
  public:
-  PlatformMonitor();
+  PlatformMonitor();            // Use freelist allocation of impl.
   ~PlatformMonitor();
+
+  static void init();           // Initialize the freelist.
+
+#else
+
+  Impl _impl;
+
+  pthread_mutex_t* mutex() { return &(_impl._mutex); }
+  pthread_cond_t* cond() { return &(_impl._cond); }
+
+ public:
+  static void init() {}         // Nothing needed for the non-indirect case.
+
+  // Default constructor and destructor.
+
+#endif // PLATFORM_MONITOR_IMPL_INDIRECT
+
+ public:
   void lock();
   void unlock();
   bool try_lock();
--- a/src/hotspot/os/posix/os_posix.inline.hpp	Tue Mar 19 14:31:52 2019 -0400
+++ b/src/hotspot/os/posix/os_posix.inline.hpp	Tue Mar 19 14:32:41 2019 -0400
@@ -50,28 +50,28 @@
 // Platform Monitor implementation
 
 inline void os::PlatformMonitor::lock() {
-  int status = pthread_mutex_lock(&_mutex);
+  int status = pthread_mutex_lock(mutex());
   assert_status(status == 0, status, "mutex_lock");
 }
 
 inline void os::PlatformMonitor::unlock() {
-  int status = pthread_mutex_unlock(&_mutex);
+  int status = pthread_mutex_unlock(mutex());
   assert_status(status == 0, status, "mutex_unlock");
 }
 
 inline bool os::PlatformMonitor::try_lock() {
-  int status = pthread_mutex_trylock(&_mutex);
+  int status = pthread_mutex_trylock(mutex());
   assert_status(status == 0 || status == EBUSY, status, "mutex_trylock");
   return status == 0;
 }
 
 inline void os::PlatformMonitor::notify() {
-  int status = pthread_cond_signal(&_cond);
+  int status = pthread_cond_signal(cond());
   assert_status(status == 0, status, "cond_signal");
 }
 
 inline void os::PlatformMonitor::notify_all() {
-  int status = pthread_cond_broadcast(&_cond);
+  int status = pthread_cond_broadcast(cond());
   assert_status(status == 0, status, "cond_broadcast");
 }