Dear Dan, I reviewed round 1. Looks good to me.
Thank you for the updated webbrew. Carsten On Tue, Oct 20, 2015 at 2:15 PM, Daniel D. Daugherty < daniel.daughe...@oracle.com> 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 >> >> >>
