RFR (XS) 8010723 - possible deadlock with SystemDictionary_lock

[Ioi Lam]

On 04/03/2013 10:07 PM, David Holmes wrote:
> On 3/04/2013 8:44 AM, Ioi Lam wrote:
>> Hi David,
>>
>> Sorry I didn't have time to incorporate your comment into the patch --
>> since it was a P2, I was in a hurry and committed the patch before
>> seeing your comments.
>
> That's okay.
>
>> It will probably be something like:
>>
>>     assert(class_loader.is_null() || !is_parallelCapable(class_loader),
>> "sanity");
>>     clean_up_shared_class(ik, class_loader, THREAD);
>>
>> But wait ... CDS would need a lot of changes if we were to allow
>> non-boot ClassLoaders to load from the CDS image (I am working on such a
>> prototype).
>>
>> Let's say C is a class in the CDS image. We have a parallel-capable
>> ClassLoader instance Foo. Two threads, T1 and T2, are trying to load the
>> same Klass C into Foo. We cannot allow both threads to work on the same
>> copy of C, so we have two choices:
>>
>>      1. Make it single-threads (T2 wait for T1)
>>
>> or
>>
>>      2. T1 loads C from CDS, but T2 loads C' from JAR file;
>>         the thread that finishes first wins. This is probably the
>>         correct way to preserve the parallel-capable property
>>         of the ClassLoader.
>>
>> In either case, if we need to clean up C due to OOM, we can guarantee
>> that no other thread will be holding a copy of C.
>>
>> So, maybe we don't need such an assert after all. What do you think?
>
> I'm not certain how loading from the shared archive is currently done, 
> but I would expect the archive itself is read-only hence no 
> concurrency issues. Using parallel-define-class two threads both 
> loading the same class will eventually be serialized due to locking in 
> the systemDictionary - at which point first to load wins and the 
> second then takes the result produced by the first. I'm not sure that 
> presence of CDS archive would/should change that story.

Part of the CDS archive is mapped RW -- specifically, all the 
InstanceKlasses are RW. The contents of a InstanceKlass is updated when 
it's being loaded (see InstanceKlass::restore_unshareable_info). So we 
cannot allow two concurrent threads to modify it at the same time.

- Ioi