--- old/src/hotspot/share/runtime/objectMonitor.cpp 2019-04-25 12:04:42.096813937 -0400 +++ new/src/hotspot/share/runtime/objectMonitor.cpp 2019-04-25 12:04:41.280813951 -0400 @@ -2099,12 +2099,16 @@ // will cause subsequent async deflation to skip it. However, // previous or concurrent async deflation is a race. if (om_ptr->_owner == DEFLATER_MARKER && om_ptr->_contentions <= 0) { - // Async deflation is in progress. Attempt to restore the - // header/dmw to the object's header so that we only retry once - // if the deflater thread happens to be slow. - om_ptr->install_displaced_markword_in_object(object); - om_ptr->dec_ref_count(); - return false; + // Async deflation is in progress. + if (om_ptr->ref_count() <= 0) { + // And our ref_count increment above lost the race to async + // deflation. Attempt to restore the header/dmw to the + // object's header so that we only retry once if the deflater + // thread happens to be slow. + om_ptr->install_displaced_markword_in_object(object); + om_ptr->dec_ref_count(); + return false; + } } // The ObjectMonitor could have been deflated and reused for // another object before we bumped the ref_count so make sure --- old/src/hotspot/share/runtime/objectMonitor.inline.hpp 2019-04-25 12:04:43.264813917 -0400 +++ new/src/hotspot/share/runtime/objectMonitor.inline.hpp 2019-04-25 12:04:42.556813929 -0400 @@ -163,7 +163,9 @@ // The decrement only needs to be MO_ACQ_REL since the reference // counter is volatile. Atomic::dec(&_ref_count); - guarantee(_ref_count >= 0, "sanity check: ref_count=%d", _ref_count); + // Can be negative as part of async deflation protocol. + guarantee(AsyncDeflateIdleMonitors || _ref_count >= 0, + "sanity check: ref_count=%d", _ref_count); } inline void ObjectMonitor::inc_ref_count() { @@ -171,7 +173,9 @@ // counter update is seen as soon as possible in a race with the // async deflation protocol. Atomic::inc(&_ref_count); - guarantee(_ref_count > 0, "sanity check: ref_count=%d", _ref_count); + // Can be negative as part of async deflation protocol. + guarantee(AsyncDeflateIdleMonitors || _ref_count > 0, + "sanity check: ref_count=%d", _ref_count); } inline jint ObjectMonitor::ref_count() const { --- old/src/hotspot/share/runtime/synchronizer.cpp 2019-04-25 12:04:44.240813900 -0400 +++ new/src/hotspot/share/runtime/synchronizer.cpp 2019-04-25 12:04:43.764813908 -0400 @@ -1175,6 +1175,15 @@ take->_header = NULL; take->set_owner(NULL); take->_contentions = 0; + + if (take->ref_count() < 0) { + // Add back max_jint to restore the ref_count field to its + // proper value. + Atomic::add(max_jint, &take->_ref_count); + + assert(take->ref_count() >= 0, "must not be negative: ref_count=%d", + take->ref_count()); + } } guarantee(!take->is_busy(), "invariant"); take->Recycle(); @@ -1776,11 +1785,13 @@ // makes contentions negative as signals to contending threads that // an async deflation is in progress. There are a number of checks // as part of the protocol to make sure that the calling thread has -// not lost the race to a contending thread. +// not lost the race to a contending thread or to a thread that just +// wants to use the ObjectMonitor*. // // The ObjectMonitor has been successfully async deflated when: -// (owner == DEFLATER_MARKER && contentions < 0). Contending threads -// that see those values know to retry their operation. +// (owner == DEFLATER_MARKER && contentions < 0 && ref_count < 0). +// Contending threads or ObjectMonitor* using threads that see those +// values know to retry their operation. // bool ObjectSynchronizer::deflate_monitor_using_JT(ObjectMonitor* mid, ObjectMonitor** freeHeadp, @@ -1815,22 +1826,28 @@ } if (Atomic::cmpxchg(-max_jint, &mid->_contentions, (jint)0) == 0) { - // Make contentions negative to force racing threads to retry. - // This is the second part of the async deflation dance. + // Make contentions negative to force any contending threads to + // retry. This is the second part of the async deflation dance. - if (mid->_owner == DEFLATER_MARKER && mid->ref_count() == 0) { + if (mid->_owner == DEFLATER_MARKER && + Atomic::cmpxchg(-max_jint, &mid->_ref_count, (jint)0) == 0) { // If owner is still DEFLATER_MARKER, then we have successfully - // signaled any racing threads to retry. If it is not, then we + // signaled any contending threads to retry. If it is not, then we // have lost the race to an entering thread and the ObjectMonitor - // is now busy. If the ObjectMonitor* is in use, then we have - // lost that race. This is the third and final part of the async - // deflation dance. + // is now busy. If we cannot make ref_count negative (because the + // ObjectMonitor* is in use), then we have lost that race instead. + // This is the third and final part of the async deflation dance. // Note: This owner check solves the ABA problem with contentions // where another thread acquired the ObjectMonitor, finished // using it and restored the contentions to zero. - // Note: This ref_count check solves the race with save_om_ptr() - // where its ref_count increment happens after the first ref_count - // check in this function and before contentions is made negative. + // Note: Making ref_count negative solves the race with + // ObjectMonitor::save_om_ptr() where its ref_count increment + // happens after the first ref_count check in this function. + // Note: Making ref_count negative must happen after the third + // part check of "owner == DEFLATER_MARKER". When save_om_ptr() + // retries, it will call install_displaced_markword_in_object() + // which will disconnect the object from the ObjectMonitor so + // deflation must happen. // Sanity checks for the races: guarantee(mid->_waiters == 0, "must be 0: waiters=%d", mid->_waiters);