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