Code Review Request for 6977738

[Mandy Chung]

  Hi David,

On 9/16/10 6:05 PM, David Holmes wrote:
> Hi Mandy,
>
> This seems quite an involved solution compared to what is described in 
> the CR. The lazy initialization of the bcp seems to be a main part of 
> the problem (or rather the requisite synchronization thereof). Do we 
> really need to lazily initialize the bcp?
>

My initial fix was to initialize the bcp outside of the synchronized 
section as part of the Launcher initialization code.  I later found that 
java.util.zip.ZipFile static initializer also gets a system property 
that will run into a deadlock if the system properties object is locked 
by another thread.

> Even if we do, can we not simply avoid the nested locking by only 
> locking when needing to initialize the bcp (and cache it in a 
> volatile) and move the getProperty outside of that synchronized 
> section. No nested locking so no deadlocks.
>

I could cache the bcp.   To fix this bug, it also need to cache 
"sun.zip.disableMemoryMapping" system property.  Instead of caching 
individual system properties, I go with caching the Properties object in 
case other part in the lib also avoids any new locking issue found in 
the future.

Mandy

> Cheers,
> David
>
>
> Mandy Chung said the following on 09/17/10 09:30:
>>
>>
>> Fix 6977738: Deadlock between java.lang.ClassLoader and 
>> java.util.Properties
>>
>> Webrev at:
>>    http://cr.openjdk.java.net/~mchung/6977738/webrev.00/
>>
>> ClassLoader.getResource locking the system properties object is
>> deadlock prone. For example, the 
>> sun.misc.Launcher.getBootstrapClassPath()
>> method is a synchronized method. A thread calling
>> ClassLoader.getResource method attempts to synchronize on
>> the system properties object while holding sun.misc.Launcher.class.
>> It will deadlock if there is another thread is holding the monitor
>> lock of the system properties object (e.g. calls the Properties.save()
>> method) while it attempts to load a resource file.  In addition,
>> ClassLoader.getResource may invoke the ZipFile initialization that
>> also needs to get the system property.
>>
>> This fix caches the system properties at initialization so that
>> the ZipFile and Launcher will read the system property from the
>> private Properties object.  In addition, I make some minor
>> changes in Properties.save and storeToXML method to confine the
>> synchronization needed and also have the DownloadManager to
>> maintain its bootclasspath.
>>
>> Thanks
>> Mandy
>>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/core-libs-dev/attachments/20100916/5b7f6b5e/attachment.html>