Request for reviews (XS): 7011386: race in objArrayKlass::array_klass_impl

[Vladimir Kozlov]

Thank you, Igor

Vladimir

Igor Veresov wrote:
> Looks good.
> 
> igor
> 
> On 1/11/11 3:48 PM, Vladimir Kozlov wrote:
>> You are right, I added store_store barriers and make these fields 
>> volatile:
>>
>> http://cr.openjdk.java.net/~kvn/7011386/webrev
>>
>> Vladimir
>>
>> Tom Rodriguez wrote:
>>> On Jan 11, 2011, at 11:15 AM, Vladimir Kozlov wrote:
>>>
>>>> Tom,
>>>>
>>>> Using oop_store() instead of oop_store_without_check()
>>>> will generate barriers but it is overkill.
>>>
>>> Those are gc barriers. I was really referring to fences. The code is a
>>> classic instance of the double check locking idiom for lazy init:
>>>
>>> objArrayKlassHandle ak (THREAD, this_oop->higher_dimension());
>>> if (ak.is_null()) {
>>> if (or_null) return NULL;
>>>
>>> ResourceMark rm;
>>> JavaThread *jt = (JavaThread *)THREAD;
>>> {
>>> MutexLocker mc(Compile_lock, THREAD); // for vtables
>>> // Ensure atomic creation of higher dimensions
>>> MutexLocker mu(MultiArray_lock, THREAD);
>>>
>>> // Check if another thread beat us
>>> ak = objArrayKlassHandle(THREAD, this_oop->higher_dimension());
>>> if( ak.is_null() ) {
>>>
>>> So doesn't that mean that we need a fence of some kind?
>>>
>>> tom
>>>
>>>> I think marking receiver as volatile should be enough:
>>>>
>>>> void set_higher_dimension(klassOop k) volatile {
>>>> oop_store_without_check((oop*) &_higher_dimension, (oop) k); }
>>>> void set_lower_dimension(klassOop k) volatile {
>>>> oop_store_without_check((oop*) &_lower_dimension, (oop) k); }
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>> Tom Rodriguez wrote:
>>>>> That look good, though I keep wondering if we need a barrier in
>>>>> between or if those fields really should be volatile. It seems like
>>>>> we're playing a little loose with the locking for these lazy values.
>>>>> tom
>>>>> On Jan 11, 2011, at 9:54 AM, Vladimir Kozlov wrote:
>>>>>> http://cr.openjdk.java.net/~kvn/7011386/webrev
>>>>>>
>>>>>> Fixed 7011386: race in objArrayKlass::array_klass_impl
>>>>>>
>>>>>> Other threads may access _lower_dimension field before
>>>>>> it is initialized by thread which holds the lock in
>>>>>> objArrayKlass::array_klass_impl().
>>>>>>
>>>>>> Move _lower_dimension field initialization before
>>>>>> _higher_dimension.
>>>
>