Code Review Request for 6977738
Mandy Chung
mandy.chung at oracle.com
Mon Sep 27 18:59:16 UTC 2010
Hi Alan,
On 09/27/10 08:33, Alan Bateman wrote:
>
> 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.
>
Thanks for reviewing it. Yes, I still need a reviewer for this fix.
> 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).
I agree that it's better to add a method to save the system properties
during initialization and make it explicit. I'll update the fix and
send out the new webrev.
> 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.
>
Properties.getProperty doesn't have permission check and thus there is
no need for a privileged block for this method. Will think about the
method name getSavedSystemProperty will work, or maybe
getSystemPropertyWithNoLock?
> 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".
>
I can clean up the above you mention.
Mandy
More information about the core-libs-dev
mailing list