RFR - 8132734: java.util.jar.* changes to support multi-release jar files
Xueming Shen
xueming.shen at oracle.com
Tue Nov 17 17:49:24 UTC 2015
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
More information about the core-libs-dev
mailing list