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

[Coleen Phillimore]

>> Right now the extra MDO would get GC'ed because nothing references it and it's in the PermGen.  But we're moving everything out of the PermGen. When it's not allocated in the PermGen, this extra MDO will get deallocated when the memory region associated with it's classloader is deallocated.  I think we should revisit this thing though at that point because it would be leaked with the bootclassloader.
>>     
>
> Freeing won't be easy in the new world since we're using pointer bumping.  We might actually need to use a lock for this so that we don't allocate wasted space, though maybe it's rare enough that we could ignore it.
>   

It's so rare right now, I can't reproduce it!  :-(  We might want to use 
C Heap allocation for MDOs and use an explicit free if this becomes a 
problem in the new world.

Coleen
> tom
>
>   
>> Thanks,
>> Coleen
>>     
>>> David
>>>
>>>       
>>>> igor
>>>>
>>>>         
>>>>> Cheers,
>>>>> David
>>>>>
>>>>>           
>>>>>> 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
>>>>>>>               
>
>