[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