--- old/src/os/bsd/vm/os_bsd.cpp Thu Jan 17 10:09:31 2013 +++ new/src/os/bsd/vm/os_bsd.cpp Thu Jan 17 10:09:31 2013 @@ -4091,11 +4091,12 @@ } -- _nParked ; - // In theory we could move the ST of 0 into _Event past the unlock(), - // but then we'd need a MEMBAR after the ST. _Event = 0 ; status = pthread_mutex_unlock(_mutex); assert_status(status == 0, status, "mutex_unlock"); + // Paranoia to ensure our locked and lock-free paths interact + // correctly with each other. + OrderAccess::fence(); } guarantee (_Event >= 0, "invariant") ; } @@ -4158,40 +4159,44 @@ status = pthread_mutex_unlock(_mutex); assert_status(status == 0, status, "mutex_unlock"); assert (_nParked == 0, "invariant") ; + // Paranoia to ensure our locked and lock-free paths interact + // correctly with each other. + OrderAccess::fence(); return ret; } void os::PlatformEvent::unpark() { - int v, AnyWaiters ; - for (;;) { - v = _Event ; - if (v > 0) { - // The LD of _Event could have reordered or be satisfied - // by a read-aside from this processor's write buffer. - // To avoid problems execute a barrier and then - // ratify the value. - OrderAccess::fence() ; - if (_Event == v) return ; - continue ; - } - if (Atomic::cmpxchg (v+1, &_Event, v) == v) break ; + // Transitions for _Event: + // 0 :=> 1 + // 1 :=> 1 + // -1 :=> either 0 or 1; must signal target thread + // That is, we can safely transition _Event from -1 to either + // 0 or 1. Forcing 1 is slightly more efficient for back-to-back + // unpark() calls. + // See also: "Semaphores in Plan 9" by Mullender & Cox + // + // Note: Forcing a transition from "-1" to "1" on an unpark() means + // that it will take two back-to-back park() calls for the owning + // thread to block. This has the benefit of forcing a spurious return + // from the first park() call after an unpark() call which will help + // shake out uses of park() and unpark() without condition variables. + + if (Atomic::xchg(1, &_Event) >= 0) return; + + // Wait for the thread associated with the event to vacate + int status = pthread_mutex_lock(_mutex); + assert_status(status == 0, status, "mutex_lock"); + int AnyWaiters = _nParked; + assert(AnyWaiters == 0 || AnyWaiters == 1, "invariant"); + if (AnyWaiters != 0 && WorkAroundNPTLTimedWaitHang) { + AnyWaiters = 0; + pthread_cond_signal(_cond); } - if (v < 0) { - // Wait for the thread associated with the event to vacate - int status = pthread_mutex_lock(_mutex); - assert_status(status == 0, status, "mutex_lock"); - AnyWaiters = _nParked ; - assert (AnyWaiters == 0 || AnyWaiters == 1, "invariant") ; - if (AnyWaiters != 0 && WorkAroundNPTLTimedWaitHang) { - AnyWaiters = 0 ; - pthread_cond_signal (_cond); - } - status = pthread_mutex_unlock(_mutex); - assert_status(status == 0, status, "mutex_unlock"); - if (AnyWaiters != 0) { - status = pthread_cond_signal(_cond); - assert_status(status == 0, status, "cond_signal"); - } + status = pthread_mutex_unlock(_mutex); + assert_status(status == 0, status, "mutex_unlock"); + if (AnyWaiters != 0) { + status = pthread_cond_signal(_cond); + assert_status(status == 0, status, "cond_signal"); } // Note that we signal() _after dropping the lock for "immortal" Events. @@ -4277,13 +4282,14 @@ } void Parker::park(bool isAbsolute, jlong time) { + // Ideally we'd do something useful while spinning, such + // as calling unpackTime(). + // Optional fast-path check: // Return immediately if a permit is available. - if (_counter > 0) { - _counter = 0 ; - OrderAccess::fence(); - return ; - } + // We depend on Atomic::xchg() having full barrier semantics + // since we are doing a lock-free update to _counter. + if (Atomic::xchg(0, &_counter) > 0) return; Thread* thread = Thread::current(); assert(thread->is_Java_thread(), "Must be JavaThread"); @@ -4324,6 +4330,8 @@ _counter = 0; status = pthread_mutex_unlock(_mutex); assert (status == 0, "invariant") ; + // Paranoia to ensure our locked and lock-free paths interact + // correctly with each other and Java-level accesses. OrderAccess::fence(); return; } @@ -4360,12 +4368,14 @@ _counter = 0 ; status = pthread_mutex_unlock(_mutex) ; assert_status(status == 0, status, "invariant") ; + // Paranoia to ensure our locked and lock-free paths interact + // correctly with each other and Java-level accesses. + OrderAccess::fence(); + // If externally suspended while waiting, re-suspend if (jt->handle_special_suspend_equivalent_condition()) { jt->java_suspend_self(); } - - OrderAccess::fence(); } void Parker::unpark() { --- old/src/os/linux/vm/os_linux.cpp Thu Jan 17 10:09:32 2013 +++ new/src/os/linux/vm/os_linux.cpp Thu Jan 17 10:09:32 2013 @@ -5001,11 +5001,12 @@ } -- _nParked ; - // In theory we could move the ST of 0 into _Event past the unlock(), - // but then we'd need a MEMBAR after the ST. _Event = 0 ; status = pthread_mutex_unlock(_mutex); assert_status(status == 0, status, "mutex_unlock"); + // Paranoia to ensure our locked and lock-free paths interact + // correctly with each other. + OrderAccess::fence(); } guarantee (_Event >= 0, "invariant") ; } @@ -5068,40 +5069,44 @@ status = pthread_mutex_unlock(_mutex); assert_status(status == 0, status, "mutex_unlock"); assert (_nParked == 0, "invariant") ; + // Paranoia to ensure our locked and lock-free paths interact + // correctly with each other. + OrderAccess::fence(); return ret; } void os::PlatformEvent::unpark() { - int v, AnyWaiters ; - for (;;) { - v = _Event ; - if (v > 0) { - // The LD of _Event could have reordered or be satisfied - // by a read-aside from this processor's write buffer. - // To avoid problems execute a barrier and then - // ratify the value. - OrderAccess::fence() ; - if (_Event == v) return ; - continue ; - } - if (Atomic::cmpxchg (v+1, &_Event, v) == v) break ; + // Transitions for _Event: + // 0 :=> 1 + // 1 :=> 1 + // -1 :=> either 0 or 1; must signal target thread + // That is, we can safely transition _Event from -1 to either + // 0 or 1. Forcing 1 is slightly more efficient for back-to-back + // unpark() calls. + // See also: "Semaphores in Plan 9" by Mullender & Cox + // + // Note: Forcing a transition from "-1" to "1" on an unpark() means + // that it will take two back-to-back park() calls for the owning + // thread to block. This has the benefit of forcing a spurious return + // from the first park() call after an unpark() call which will help + // shake out uses of park() and unpark() without condition variables. + + if (Atomic::xchg(1, &_Event) >= 0) return; + + // Wait for the thread associated with the event to vacate + int status = pthread_mutex_lock(_mutex); + assert_status(status == 0, status, "mutex_lock"); + int AnyWaiters = _nParked; + assert(AnyWaiters == 0 || AnyWaiters == 1, "invariant"); + if (AnyWaiters != 0 && WorkAroundNPTLTimedWaitHang) { + AnyWaiters = 0; + pthread_cond_signal(_cond); } - if (v < 0) { - // Wait for the thread associated with the event to vacate - int status = pthread_mutex_lock(_mutex); - assert_status(status == 0, status, "mutex_lock"); - AnyWaiters = _nParked ; - assert (AnyWaiters == 0 || AnyWaiters == 1, "invariant") ; - if (AnyWaiters != 0 && WorkAroundNPTLTimedWaitHang) { - AnyWaiters = 0 ; - pthread_cond_signal (_cond); - } - status = pthread_mutex_unlock(_mutex); - assert_status(status == 0, status, "mutex_unlock"); - if (AnyWaiters != 0) { - status = pthread_cond_signal(_cond); - assert_status(status == 0, status, "cond_signal"); - } + status = pthread_mutex_unlock(_mutex); + assert_status(status == 0, status, "mutex_unlock"); + if (AnyWaiters != 0) { + status = pthread_cond_signal(_cond); + assert_status(status == 0, status, "cond_signal"); } // Note that we signal() _after dropping the lock for "immortal" Events. @@ -5187,13 +5192,14 @@ } void Parker::park(bool isAbsolute, jlong time) { + // Ideally we'd do something useful while spinning, such + // as calling unpackTime(). + // Optional fast-path check: // Return immediately if a permit is available. - if (_counter > 0) { - _counter = 0 ; - OrderAccess::fence(); - return ; - } + // We depend on Atomic::xchg() having full barrier semantics + // since we are doing a lock-free update to _counter. + if (Atomic::xchg(0, &_counter) > 0) return; Thread* thread = Thread::current(); assert(thread->is_Java_thread(), "Must be JavaThread"); @@ -5234,6 +5240,8 @@ _counter = 0; status = pthread_mutex_unlock(_mutex); assert (status == 0, "invariant") ; + // Paranoia to ensure our locked and lock-free paths interact + // correctly with each other and Java-level accesses. OrderAccess::fence(); return; } @@ -5270,12 +5278,14 @@ _counter = 0 ; status = pthread_mutex_unlock(_mutex) ; assert_status(status == 0, status, "invariant") ; + // Paranoia to ensure our locked and lock-free paths interact + // correctly with each other and Java-level accesses. + OrderAccess::fence(); + // If externally suspended while waiting, re-suspend if (jt->handle_special_suspend_equivalent_condition()) { jt->java_suspend_self(); } - - OrderAccess::fence(); } void Parker::unpark() { --- old/src/os/solaris/vm/os_solaris.cpp Thu Jan 17 10:09:34 2013 +++ new/src/os/solaris/vm/os_solaris.cpp Thu Jan 17 10:09:33 2013 @@ -6014,6 +6014,9 @@ _Event = 0 ; status = os::Solaris::mutex_unlock(_mutex); assert_status(status == 0, status, "mutex_unlock"); + // Paranoia to ensure our locked and lock-free paths interact + // correctly with each other. + OrderAccess::fence(); } } @@ -6055,51 +6058,43 @@ _Event = 0 ; status = os::Solaris::mutex_unlock(_mutex); assert_status(status == 0, status, "mutex_unlock"); + // Paranoia to ensure our locked and lock-free paths interact + // correctly with each other. + OrderAccess::fence(); return ret; } void os::PlatformEvent::unpark() { - int v, AnyWaiters; + // Transitions for _Event: + // 0 :=> 1 + // 1 :=> 1 + // -1 :=> either 0 or 1; must signal target thread + // That is, we can safely transition _Event from -1 to either + // 0 or 1. Forcing 1 is slightly more efficient for back-to-back + // unpark() calls. + // See also: "Semaphores in Plan 9" by Mullender & Cox + // + // Note: Forcing a transition from "-1" to "1" on an unpark() means + // that it will take two back-to-back park() calls for the owning + // thread to block. This has the benefit of forcing a spurious return + // from the first park() call after an unpark() call which will help + // shake out uses of park() and unpark() without condition variables. - // Increment _Event. - // Another acceptable implementation would be to simply swap 1 - // into _Event: - // if (Swap (&_Event, 1) < 0) { - // mutex_lock (_mutex) ; AnyWaiters = nParked; mutex_unlock (_mutex) ; - // if (AnyWaiters) cond_signal (_cond) ; - // } + if (Atomic::xchg(1, &_Event) >= 0) return; - for (;;) { - v = _Event ; - if (v > 0) { - // The LD of _Event could have reordered or be satisfied - // by a read-aside from this processor's write buffer. - // To avoid problems execute a barrier and then - // ratify the value. A degenerate CAS() would also work. - // Viz., CAS (v+0, &_Event, v) == v). - OrderAccess::fence() ; - if (_Event == v) return ; - continue ; - } - if (Atomic::cmpxchg (v+1, &_Event, v) == v) break ; - } - // If the thread associated with the event was parked, wake it. - if (v < 0) { - int status ; - // Wait for the thread assoc with the PlatformEvent to vacate. - status = os::Solaris::mutex_lock(_mutex); - assert_status(status == 0, status, "mutex_lock"); - AnyWaiters = _nParked ; - status = os::Solaris::mutex_unlock(_mutex); - assert_status(status == 0, status, "mutex_unlock"); - guarantee (AnyWaiters == 0 || AnyWaiters == 1, "invariant") ; - if (AnyWaiters != 0) { - // We intentional signal *after* dropping the lock - // to avoid a common class of futile wakeups. - status = os::Solaris::cond_signal(_cond); - assert_status(status == 0, status, "cond_signal"); - } + // Wait for the thread assoc with the PlatformEvent to vacate. + int status = os::Solaris::mutex_lock(_mutex); + assert_status(status == 0, status, "mutex_lock"); + int AnyWaiters = _nParked; + status = os::Solaris::mutex_unlock(_mutex); + assert_status(status == 0, status, "mutex_unlock"); + guarantee(AnyWaiters == 0 || AnyWaiters == 1, "invariant"); + if (AnyWaiters != 0) { + // We intentional signal *after* dropping the lock + // to avoid a common class of futile wakeups. + status = os::Solaris::cond_signal(_cond); + assert_status(status == 0, status, "cond_signal"); } } @@ -6177,14 +6172,14 @@ } void Parker::park(bool isAbsolute, jlong time) { + // Ideally we'd do something useful while spinning, such + // as calling unpackTime(). // Optional fast-path check: // Return immediately if a permit is available. - if (_counter > 0) { - _counter = 0 ; - OrderAccess::fence(); - return ; - } + // We depend on Atomic::xchg() having full barrier semantics + // since we are doing a lock-free update to _counter. + if (Atomic::xchg(0, &_counter) > 0) return; // Optional fast-exit: Check interrupt before trying to wait Thread* thread = Thread::current(); @@ -6226,6 +6221,8 @@ _counter = 0; status = os::Solaris::mutex_unlock(_mutex); assert (status == 0, "invariant") ; + // Paranoia to ensure our locked and lock-free paths interact + // correctly with each other and Java-level accesses. OrderAccess::fence(); return; } @@ -6267,12 +6264,14 @@ _counter = 0 ; status = os::Solaris::mutex_unlock(_mutex); assert_status(status == 0, status, "mutex_unlock") ; + // Paranoia to ensure our locked and lock-free paths interact + // correctly with each other and Java-level accesses. + OrderAccess::fence(); // If externally suspended while waiting, re-suspend if (jt->handle_special_suspend_equivalent_condition()) { jt->java_suspend_self(); } - OrderAccess::fence(); } void Parker::unpark() { --- old/src/os/windows/vm/os_windows.cpp Thu Jan 17 10:09:35 2013 +++ new/src/os/windows/vm/os_windows.cpp Thu Jan 17 10:09:35 2013 @@ -4565,6 +4565,7 @@ } v = _Event ; _Event = 0 ; + // see comment at end of os::PlatformEvent::park() below: OrderAccess::fence() ; // If we encounter a nearly simultanous timeout expiry and unpark() // we return OS_OK indicating we awoke via unpark(). @@ -4602,25 +4603,25 @@ void os::PlatformEvent::unpark() { guarantee (_ParkHandle != NULL, "Invariant") ; - int v ; - for (;;) { - v = _Event ; // Increment _Event if it's < 1. - if (v > 0) { - // If it's already signaled just return. - // The LD of _Event could have reordered or be satisfied - // by a read-aside from this processor's write buffer. - // To avoid problems execute a barrier and then - // ratify the value. A degenerate CAS() would also work. - // Viz., CAS (v+0, &_Event, v) == v). - OrderAccess::fence() ; - if (_Event == v) return ; - continue ; - } - if (Atomic::cmpxchg (v+1, &_Event, v) == v) break ; - } - if (v < 0) { - ::SetEvent (_ParkHandle) ; - } + + // Transitions for _Event: + // 0 :=> 1 + // 1 :=> 1 + // -1 :=> either 0 or 1; must signal target thread + // That is, we can safely transition _Event from -1 to either + // 0 or 1. Forcing 1 is slightly more efficient for back-to-back + // unpark() calls. + // See also: "Semaphores in Plan 9" by Mullender & Cox + // + // Note: Forcing a transition from "-1" to "1" on an unpark() means + // that it will take two back-to-back park() calls for the owning + // thread to block. This has the benefit of forcing a spurious return + // from the first park() call after an unpark() call which will help + // shake out uses of park() and unpark() without condition variables. + + if (Atomic::xchg(1, &_Event) >= 0) return; + + ::SetEvent(_ParkHandle); } --- old/src/share/vm/classfile/javaClasses.cpp Thu Jan 17 10:09:37 2013 +++ new/src/share/vm/classfile/javaClasses.cpp Thu Jan 17 10:09:36 2013 @@ -924,7 +924,6 @@ // Write the thread status value to threadStatus field in java.lang.Thread java class. void java_lang_Thread::set_thread_status(oop java_thread, java_lang_Thread::ThreadStatus status) { - assert(JavaThread::current()->thread_state() == _thread_in_vm, "Java Thread is not running in vm"); // The threadStatus is only present starting in 1.5 if (_thread_status_offset > 0) { java_thread->int_field_put(_thread_status_offset, status); --- old/src/share/vm/runtime/objectMonitor.cpp Thu Jan 17 10:09:38 2013 +++ new/src/share/vm/runtime/objectMonitor.cpp Thu Jan 17 10:09:38 2013 @@ -653,8 +653,7 @@ assert (_succ != Self, "invariant") ; if (_Responsible == Self) { _Responsible = NULL ; - // Dekker pivot-point. - // Consider OrderAccess::storeload() here + OrderAccess::fence(); // Dekker pivot-point // We may leave threads on cxq|EntryList without a designated // "Responsible" thread. This is benign. When this thread subsequently @@ -674,10 +673,6 @@ // // The MEMBAR, above, prevents the LD of cxq|EntryList in the subsequent // exit operation from floating above the ST Responsible=null. - // - // In *practice* however, EnterI() is always followed by some atomic - // operation such as the decrement of _count in ::enter(). Those atomics - // obviate the need for the explicit MEMBAR, above. } // We've acquired ownership with CAS(). --- old/src/share/vm/runtime/objectMonitor.inline.hpp Thu Jan 17 10:09:39 2013 +++ new/src/share/vm/runtime/objectMonitor.inline.hpp Thu Jan 17 10:09:39 2013 @@ -1,5 +1,5 @@ /* - * Copyright (c) 1998, 2010, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1998, 2012, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -101,10 +101,12 @@ return _count; } +// Do NOT set _count = 0. There is a race such that _count could +// be set while inflating prior to setting _owner +// Just use Atomic::inc/dec and assert 0 when monitor put on free list inline void ObjectMonitor::set_owner(void* owner) { _owner = owner; _recursions = 0; - _count = 0; } --- old/src/share/vm/runtime/synchronizer.cpp Thu Jan 17 10:09:40 2013 +++ new/src/share/vm/runtime/synchronizer.cpp Thu Jan 17 10:09:40 2013 @@ -333,7 +333,9 @@ void ObjectSynchronizer::jni_exit(oop obj, Thread* THREAD) { TEVENT (jni_exit) ; if (UseBiasedLocking) { - BiasedLocking::revoke_and_rebias(obj, false, THREAD); + Handle h_obj(THREAD, obj); + BiasedLocking::revoke_and_rebias(h_obj, false, THREAD); + obj = h_obj(); } assert(!obj->mark()->has_bias_pattern(), "biases should be revoked by now"); --- old/src/share/vm/runtime/vmThread.cpp Thu Jan 17 10:09:41 2013 +++ new/src/share/vm/runtime/vmThread.cpp Thu Jan 17 10:09:41 2013 @@ -570,7 +570,11 @@ if (!t->is_VM_thread()) { SkipGCALot sgcalot(t); // avoid re-entrant attempts to gc-a-lot // JavaThread or WatcherThread - t->check_for_valid_safepoint_state(true); + bool concurrent = op->evaluate_concurrently(); + // only blocking VM operations need to verify the caller's safepoint state: + if (!concurrent) { + t->check_for_valid_safepoint_state(true); + } // New request from Java thread, evaluate prologue if (!op->doit_prologue()) { @@ -582,7 +586,6 @@ // It does not make sense to execute the epilogue, if the VM operation object is getting // deallocated by the VM thread. - bool concurrent = op->evaluate_concurrently(); bool execute_epilog = !op->is_cheap_allocated(); assert(!concurrent || op->is_cheap_allocated(), "concurrent => cheap_allocated");