RFR - 8132734: java.util.jar.* changes to support multi-release jar files

[Xueming Shen]

On 11/11/15 8:44 AM, Steve Drach wrote:
> Hi,
>
> Please review the new webrev that addresses the issues raised by 
> Sherman and Alan in the last iteration.  In particular:
> - fixed the race condition in isMultiRelease() and another one with 
> the variables ‘version’ and ‘configured’
> - changed the fragment for JarURLConnection runtime versioning from 
> ‘rtversioned’ to ‘runtime’, and created documentation for it
> - used try with resources to open JarFile in all the tests
>
> Issue: https://bugs.openjdk.java.net/browse/JDK-8132734
> JEP 238: https://bugs.openjdk.java.net/browse/JDK-8047305
> Webrev: 
> http://cr.openjdk.java.net/~psandoz/multiversion-jar/jar-webrev/ 
> <http://cr.openjdk.java.net/%7Epsandoz/multiversion-jar/jar-webrev/>
>
>

Hi Steve,

Seems like the "version" is now a little confused.

(1) getVersioned() says "or 0 if this jar file is processed as if it is 
an unversioned or is
not a multi-release jar file". The implementation now returns 8 
(BASE_VERSION is
8 now) if not a multi-release jar (?).

(2) setVersioned() says "If the specified version is 0 then this is 
configured to be an
unversioned jar file". The implementation seems treat anything < 
BASE_VERSION as
such? Sure, technically it's still correct. Given the BASE_VERSION is a 
private field, any
thing between 0 and BASE_VERSION becomes unspecified gray area.

(3) getRuntimeVersionedEntry() spec says "If the JarFile is a 
multi-release jar file and is
configured to be processed as such, then ...". However the implementation

if (RUNTIME_VERSION > BASE_VERSION && !name.startsWith(META_INF) && 
isMultiRelease()) {
   ...
}

does not appears to check the logic "is configured to be ..."?

And

391 if (manifestRead) {
392 return isMultiRelease;
393 }
394 synchronized (this) {
395 // this needs to be set prior to getManifest() call to prevent 
recursive loop
396 manifestRead = true;
397 try {
398 Manifest man = getManifest();
399 isMultiRelease = (man != null) && 
man.getMainAttributes().containsKey(MULTI_RELEASE);
400 } catch (IOException x) {
401 isMultiRelease = false;
402 }
403 }
404 return isMultiRelease;

don't we have a race condition if #391/2 is outside the synchronized 
block? for sample, one
thread is inside sync block and set the manifestRead = true, but before 
isMultiRelease = true/false,
and the second thread gets to #391.

thanks,
-Sherman