[modules-dev] [Fwd: [Fwd: Review request: 6563535, URLRepository.install]]

Stanley M. Ho Stanley.Ho at Sun.COM
Wed Aug 15 14:20:23 PDT 2007


Looks good. Comments inline:

- src/share/classes/sun/module/repository/RepositoryContents.java

83             if (m.hashCode() == mai.hashCode()) {

It is unclear why you want to compare hashcode instead of simply using 
equals().

- src/share/classes/sun/module/repository/URLRepository.java

>> 688             contents.put(mai, installedMD);
>>
>> If exception is thrown in the try/catch block, the contents will be 
>> updated incorrectly. Even contents is updated again in the catch 
>> block, there is still a small window that contents does not represent 
>> the correct information. Instead, this should be updated in a finally 
>> block.
> 
> If it's in the finally block, it will always get updated.  But if an 
> exception is thrown, then we don't want it to get updated, which is what 
> we need.
> 
> Or do you mean there should be a try/catch within the catch of the 
> IOException, so that contents.remove(mai) in a finally?  I made this 
> change; please let me know if it meets your intent.

I meant that the contents should not be updated at all until the entire 
install operation succeeds. Otherwise, if a thread calls into 
Repository.list() in the meantime, it will get a list of 
ModuleArchiveInfo but some of the ModuleArchiveInfo might not yet been 
installed successfully (and the installation might actually fail).

- Stanley



More information about the modules-dev mailing list