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

[serguei.spitsyn at oracle.com]

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.

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
>>
>>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20151020/4c634bae/attachment.html>