Request for review: 6922246: Deadlock during intensive class loading in parallel

[Igor Veresov]

David,

There's no question a store-store barrier is needed after 291 for 
non-TSO if we were to make it lock-free (with the current code monitor 
lock should contain such barrier anyway).

But why do you think it needs barrier before 286? There seems to be 
nothing wrong in allocating an extra MDO... Moreover, the code is 
written under this assumption.

igor

On 10/19/10 3:59 PM, David Holmes wrote:
> Hi Coleen,
>
> coleen phillimore said the following on 10/20/10 08:11:
>> Summary: allocate methodData before taking out the MethodData_lock
>> open webrev at http://cr.openjdk.java.net/~coleenp/6922246/
>> bug link at http://bugs.sun.com/view_bug.do?bug_id=6922246
>>
>> Thanks to Ramki for the diagnosis and suggestion for the fix.
>
> This is a variation on the unsafe double-checked-locking idiom.
>
> 286 if (method->method_data() == NULL) {
> 291 methodDataOop method_data = oopFactory::new_methodData(method, CHECK);
> 292 MutexLocker ml(MethodData_lock, THREAD);
> 295 if (method->method_data() == NULL) {
> 296 method->set_method_data(method_data);
>
> In general terms any stores associated with the method_data construction
> are not guaranteed to be visible by the time set_method_data is called.
> A second thread could then see a non-null method->method_data() but
> access uninitialized method_data values.
>
> A general solution would need memory barriers before 286 and after 291
> to ensure correctness on all platforms. A simpler, though perhaps
> heavier solution would be to use a MutexUnlocker around the allocation:
>
> MutexLocker ml(MethodData_lock, THREAD);
> if (method->method_data() == NULL) {
> methodDataOop method_data = NULL;
> { // need new block for scope
> MutexUnLocker ml(MethodData_lock, THREAD);
> method_data = oopFactory::new_methodData(method, CHECK);
> }
> // someone might have beaten us to it
> if (method->method_data() == NULL) {
> method->set_method_data(method_data);
> }
> }
>
> A lock-less solution as Tom proposed would also require suitable
> memory-barriers.
>
> David