Code Review Request for 6977738

Mandy Chung mandy.chung at oracle.com
Thu Sep 30 17:46:27 UTC 2010


  On 09/30/10 07:18, Alan Bateman wrote:
> Mandy Chung wrote:
>>  Alan, David,
>>
>> I revise the fix including the following:
>> 1. Use the lazy initialization holder class idiom to initialize the bcp
>> variable to simplify the synchronization.
>>
>> 2. Group the initialization of system properties in a new private method
>> System.initializeSystemProperties so that it can save a copy for 
>> internal
>> implementation use to address the deadlock issue and remove the
>> properties that are intended for private internal use from public access
>> before initializing the System.props.  This removes the 2 calls to
>> System.setProperties.
>>
>> 3. Reorder sun.net.* in FILES_java.gmk
>>
>> The new webrev is at:
>>    http://cr.openjdk.java.net/~mchung/6977738/webrev.01/
> This looks much better (and thanks for doing the various 
> drive-by-clean-ups).
>
> I assume BootClassPathHolder should be private, and that the bcp field 
> can be final.
>
> Minor nit but but the IllegalStateExceptions thrown by 
> VM.getSavedProperty capitalizes the first letter of the message 
> whereas VM.saveAndRemoveProperties doesn't.
>
> I wonder if it would be worth seeing if saveAndRemoveProperties could 
> grab the value of the java.lang.Integer.IntegerCache.high property. 
> That would allow us to be handle it more consistently with these other 
> properties that we save during initialization. The cache 
> initialization could then use VM.getSavedProperty to get the value. 
> Might be worth thinking about but you would need to be careful to 
> ensure that Integer.valueOf isn't called as otherwise isn't not going 
> to work.
>

Yes, I also thought of that.  I was trying to keep the change as 
minimal.   I agree that I should take this chance to clean up and handle 
private system properties consistently.   It's good not to leak the 
private internal interfaces for public access.

Joe, Sherman,

I change IntegerCache and ZipFile initialization to get the property 
value from the private system properties object (not calling 
System.getProperty).  I also remove the "sun.zip.disableMemoryMapping" 
property from the public system properties.  Can you take a look?

    http://cr.openjdk.java.net/~mchung/6977738/webrev.02

Thanks
Mandy



More information about the core-libs-dev mailing list