RFR (XS) 8047212: fix race between ObjectMonitor alloc and verification code (original) (raw)

Daniel D. Daugherty daniel.daugherty at oracle.com
Wed Oct 21 02:35:31 UTC 2015

Serguei,

Thanks for the quick review!

On 10/20/15, 6:43 PM, serguei.spitsyn at oracle.com wrote:

Dan,

Nice finding!

Thank you!

The fixes look good to me.

Thanks!

The changes in the oopsdo() and deflateidlemonitors() are not necessary as these functions are called in VMOperation's. But some unification with OrderAccess::loadptracquire() does not harm either.

Yes I was shooting for consistency. I didn't want someone copying the code that is run at a safepoint and "safe" to somewhere else that is not quite so safe...

Again, thanks for the fast review!

Dan

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::loadptracquire() code style cleanups: - only cleaned up the functions that I touched to make the OrderAccess::loadptracquire() 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::verifyobjmonisinpool(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::releasestoreptr(&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

More information about the hotspot-runtime-dev mailing list