Code Review Request for 6977738

Alan Bateman Alan.Bateman at oracle.com
Mon Sep 27 15:33:48 UTC 2010


Mandy Chung wrote:
>
>
> 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
>
I don't know if you are still looking for a reviewer for this one. I 
agree with David's comment that it is rather complicated.

I wonder if we better to add VM.saveSystemProperties rather than having 
VM.booted do it as a side effect. The saveSystemProperties method could 
throw IllegalStateException if booted, to catch any mis-use after VM 
initialization. It could also filter out and remove properties that 
shouldn't be visible to applications, like sun.nio.MaxDirectMemorySize 
(on that property, you could change the maxDirectMemory method so that 
it doesn't need to use setProperties and could get you back the cost of 
saving the properties).  Also, on this, VM.getSystemProperty probably 
needs a better name, maybe getSavedSystemProperty, to make it clear that 
it's not the same as System.getProperty. On that, do we have any cases 
where it will be invoked after initialization has completed? I'm just 
wondering if it needs a privileged block as these have been removed from 
the callers.

In passing, I see that make/java/java/FILES_java.gmk has sun.net.www 
classes listed in the middle of the sun.misc list. It might be good to 
re-order these while you are there.

Not wishing to add to your load, but I see XMLUtils.save is using raw 
types and maybe it could be updated while you are there (if you have 
time of course).

Typo in DownloadManager at L667 -> "How" instead of "Now".

-Alan.





More information about the core-libs-dev mailing list