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