On 21/10/2015 10:43 AM, serguei.spit...@oracle.com wrote:
Dan,
Nice finding!
The fixes look good to me.
The changes in the oops_do() and deflate_idle_monitors() are not necessary
as these functions are called in VM_Operation's.
But some unification with OrderAccess::load_ptr_acquire() does not harm
either.
I was thinking on the same lines: that reading the variable at a
safepoint should already be safe, but that consistently using
load_acquire_ptr is harmless and stylistically a 'Good Thing".
The other thought I had was perhaps making the verification routines
(which are non-product) use the lock as well. :)
Aside: did you check that all the other global monitor variables are
accessed correctly - ie either always with the lock held or using
OrderAccess operations if lock-free?
I have still have the query about the vmStructs changes but otherwise
thumbs up.
Thanks,
David
Thanks,
Serguei
On 10/20/15 14:15, Daniel D. Daugherty wrote:
Greetings,
I've updated the fix based on feedback from Carsten V and David H.
Webrev URL:
http://cr.openjdk.java.net/~dcubed/8047212-webrev/1-jdk9-hs-rt/
Changes relative to round 0:
- only src/share/vm/runtime/synchronizer.cpp has changed
- reads of gBlockList now use OrderAccess::load_ptr_acquire()
code style cleanups:
- only cleaned up the functions that I touched to make the
OrderAccess::load_ptr_acquire() changes
- changed implied booleans into real boolean expressions
- moved some locals to narrower context
- added/removed some blank lines
- made casts consistent with the majority style in this file
I'm repeating all of the same testing that I did for round 0. The
round 1 bits have not yet made it through JPRT-west, but the jobs
are mostly done.
Thanks, in advance, for any comments, questions or suggestions.
Dan
On 10/19/15, 9:02 PM, Daniel D. Daugherty wrote:
Greetings,
I have a fix for a long standing race between the lock-free
ObjectMonitor
verification code and the normal (locked) ObjectMonitor block allocation
code path. For this fix, I would like at least a Runtime team reviewer
and a Serviceability team reviewer. Thanks!
JDK-8047212 runtime/ParallelClassLoading/bootstrap/random/inner-complex
assert(ObjectSynchronizer::verify_objmon_isinpool(inf)) failed:
monitor is invalid
https://bugs.openjdk.java.net/browse/JDK-8047212
Webrev URL:
http://cr.openjdk.java.net/~dcubed/8047212-webrev/0-jdk9-hs-rt/
Testing: Aurora Adhoc RT-SVC nightly batch
4 inner-complex fastdebug parallel runs for 4+ days and
600K iterations without seeing this failure; the experiment
is still running; final results to be reported at the end
of the review cycle
JPRT -testset hotspot
This fix:
- makes ObjectMonitor::gBlockList volatile
- uses "OrderAccess::release_store_ptr(&gBlockList, temp)" to
make sure the new block updates _happen before_ gBlockList is
changed to refer to the new block
- add SA support for a "static pointer volatile" field like:
static ObjectMonitor * volatile gBlockList;
See the following link for a nice description of what "volatile"
means in the different positions on a variable/parameter decl line:
http://www.embedded.com/electronics-blogs/beginner-s-corner/4023801/Introduction-to-the-Volatile-Keyword
Thanks, in advance, for any comments, questions or suggestions.
Dan
