RFR (XS) 8010723 - possible deadlock with SystemDictionary_lock

[Ioi Lam]

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.

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?

Thanks
- Ioi


On 04/01/2013 07:08 PM, David Holmes wrote:
> Is there any way to express this:
>
> 839   // This need to be re-thought when parallel-capable non-boot
> 840   // classloaders are supported by CDS (today they're not).
>
> as an assert?
>
> David
>
> On 30/03/2013 5:26 AM, Ioi Lam wrote:
>> Coleen,
>>
>> I have updated the webrev to incorporate your comments:
>>
>> http://cr.openjdk.java.net/~iklam/8010723/sysdic_lock_002/
>>
>> Thanks
>> - Ioi
>>
>> On 03/29/2013 10:38 AM, Coleen Phillimore wrote:
>>>> // This must be done outside of the SystemDictionary_lock to
>>>>     // avoid deadlock.
>>>>     //
>>>>     // Note that Klass::restore_unshareable_info (called via
>>>>     // load_instance_class above) is also called outside
>>>>     // of SystemDictionary_lock. All other threads that are
>>>>     // trying to load this class in the boot classloader
>>>>     // are already blocked above at
>>>>     // if (class_loader.is_null()) {SystemDictionary_lock->wait();}
>>> How about
>>>
>>>       // Other threads are blocked from loading this class because
>>> they are
>>>       // are waiting on the SystemDictionary_lock until this thread
>>> removes
>>>       // the placeholder below.
>>>
>>> thanks,
>>> Coleen
>>>>     //
>>>>     // This need to be re-thought when parallel-capable non-boot
>>>>     // classloaders are supported by CDS (today they're not).
>>