Withdrawn: 6957241: ClassLoader.getResources() returns only 1 instance when using jar indexing
On Tue, 31 Aug 2021 12:11:46 GMT, wxiang
wrote:
> Using jarIndex for Hibench, there is an unexpected behavior with the
> exception "Exception in thread "main"
> org.apache.hadoop.fs.UnsupportedFileSystemException: No FileSystem for scheme
> "hdfs"".
>
> After investigating it, it is related to the usage of ServiceLoader with
> JarIndex.
> The below stack shows the issue with JDK11:
>
> getResource:1016, URLClassPath$JarLoader (jdk.internal.loader)
> getResource:937, URLClassPath$JarLoader (jdk.internal.loader)
> findResource:912, URLClassPath$JarLoader (jdk.internal.loader)
> next:341, URLClassPath$1 (jdk.internal.loader)
> hasMoreElements:351, URLClassPath$1 (jdk.internal.loader)
> hasNext:355, BuiltinClassLoader$1 (jdk.internal.loader)
> hasMoreElements:363, BuiltinClassLoader$1 (jdk.internal.loader)
> next:3032, CompoundEnumeration (java.lang)
> hasMoreElements:3041, CompoundEnumeration (java.lang)
> nextProviderClass:1203, ServiceLoader$LazyClassPathLookupIterator (java.util)
> hasNextService:1221, ServiceLoader$LazyClassPathLookupIterator (java.util)
> hasNext:1265, ServiceLoader$LazyClassPathLookupIterator (java.util)
> hasNext:1300, ServiceLoader$2 (java.util)
> hasNext:1385, ServiceLoader$3 (java.util)
>
> The below API tries to get all the resources with the same name.
>
> public Enumeration findResources(final String name,
> final boolean check)
> ```
> After using JarIndex, URLClassPath.findResources only returns 1 URL.
> It is the same as the description in JDK-6957241.
>
> The issue still exists in JDK18.
>
> Root cause:
>
> public Enumeration findResources(final String name,
> final boolean check) {
> return new Enumeration<>() {
> private int index = 0;
> private URL url = null;
>
> private boolean next() {
> if (url != null) {
> return true;
> } else {
> Loader loader;
> while ((loader = getLoader(index++)) != null) {
> url = loader.findResource(name, check);
> if (url != null) {
> return true;
> }
> }
> return false;
> }
> }
> ...
> };
> }
>
> With the JarIndex, there is only one loader which is corresponding to the jar
> with the index due to the implementation in JarLoader.getResource(final
> String name, boolean check, Set visited).
>
> Loaders corresponding to other jar packages will not appear in this while.
> So it only returns 1 instance.
>
> To solve the issue, I change the implementation "private boolean next()".
> If the loader has index, traverse the index and get all the resource from the
> loader.
This pull request has been closed without being integrated.
-
PR: https://git.openjdk.java.net/jdk/pull/5316
Re: RFR: 6957241: ClassLoader.getResources() returns only 1 instance when using jar indexing
On Tue, 31 Aug 2021 12:11:46 GMT, wxiang
wrote:
> Using jarIndex for Hibench, there is an unexpected behavior with the
> exception "Exception in thread "main"
> org.apache.hadoop.fs.UnsupportedFileSystemException: No FileSystem for scheme
> "hdfs"".
>
> After investigating it, it is related to the usage of ServiceLoader with
> JarIndex.
> The below stack shows the issue with JDK11:
>
> getResource:1016, URLClassPath$JarLoader (jdk.internal.loader)
> getResource:937, URLClassPath$JarLoader (jdk.internal.loader)
> findResource:912, URLClassPath$JarLoader (jdk.internal.loader)
> next:341, URLClassPath$1 (jdk.internal.loader)
> hasMoreElements:351, URLClassPath$1 (jdk.internal.loader)
> hasNext:355, BuiltinClassLoader$1 (jdk.internal.loader)
> hasMoreElements:363, BuiltinClassLoader$1 (jdk.internal.loader)
> next:3032, CompoundEnumeration (java.lang)
> hasMoreElements:3041, CompoundEnumeration (java.lang)
> nextProviderClass:1203, ServiceLoader$LazyClassPathLookupIterator (java.util)
> hasNextService:1221, ServiceLoader$LazyClassPathLookupIterator (java.util)
> hasNext:1265, ServiceLoader$LazyClassPathLookupIterator (java.util)
> hasNext:1300, ServiceLoader$2 (java.util)
> hasNext:1385, ServiceLoader$3 (java.util)
>
> The below API tries to get all the resources with the same name.
>
> public Enumeration findResources(final String name,
> final boolean check)
> ```
> After using JarIndex, URLClassPath.findResources only returns 1 URL.
> It is the same as the description in JDK-6957241.
>
> The issue still exists in JDK18.
>
> Root cause:
>
> public Enumeration findResources(final String name,
> final boolean check) {
> return new Enumeration<>() {
> private int index = 0;
> private URL url = null;
>
> private boolean next() {
> if (url != null) {
> return true;
> } else {
> Loader loader;
> while ((loader = getLoader(index++)) != null) {
> url = loader.findResource(name, check);
> if (url != null) {
> return true;
> }
> }
> return false;
> }
> }
> ...
> };
> }
>
> With the JarIndex, there is only one loader which is corresponding to the jar
> with the index due to the implementation in JarLoader.getResource(final
> String name, boolean check, Set visited).
>
> Loaders corresponding to other jar packages will not appear in this while.
> So it only returns 1 instance.
>
> To solve the issue, I change the implementation "private boolean next()".
> If the loader has index, traverse the index and get all the resource from the
> loader.
JarIndex support will be deleted. close the PR.
-
PR: https://git.openjdk.java.net/jdk/pull/5316
Withdrawn: 8273401: Remove JarIndex support in URLClassPath
On Tue, 7 Sep 2021 03:34:27 GMT, wxiang wrote: > There is a bug for URLClassPath.findResources with JarIndex. > With some discussions about the bug, the current priority is to remove the > JAR index support in URLClassPath, > and don’t need to do anything to the jar tool in the short term, except just > to move JarIndex to the jdk.jartool module. > > The PR includes: > 1. remove the JarIndex support in URLClassPath > 2. move JarIndex into jdk.jartool module. This pull request has been closed without being integrated. - PR: https://git.openjdk.java.net/jdk/pull/5383
Re: RFR: 8273401: Remove JarIndex support in URLClassPath [v2]
On Wed, 8 Sep 2021 06:22:38 GMT, wxiang wrote: >> There is a bug for URLClassPath.findResources with JarIndex. >> With some discussions about the bug, the current priority is to remove the >> JAR index support in URLClassPath, >> and don’t need to do anything to the jar tool in the short term, except just >> to move JarIndex to the jdk.jartool module. >> >> The PR includes: >> 1. remove the JarIndex support in URLClassPath >> 2. move JarIndex into jdk.jartool module. > > wxiang has updated the pull request incrementally with one additional commit > since the last revision: > > Some minor changes > > Include: > 1. remove public for INDEX_NAME in JarFile > 2. recover the definition for INDEX_NAME in JarIndex > 3. mv the test case JarIndexMergeTest.java into test/jdk/java/util/jar Firstly, disable JarIndex support in URLClassPath. So close the PR. - PR: https://git.openjdk.java.net/jdk/pull/5383
Integrated: 8273401: Disable JarIndex support in URLClassPath
On Wed, 15 Sep 2021 09:33:10 GMT, wxiang wrote: > There is a bug for URLClassPath.findResources with JarIndex. > Currently, there was agreement on dropping the support from the > URLClassLoader implementation but it was suggested that it should be disabled > for a release or two before the code is removed. > A system property can be used to re-enable JarIndex support in URLClassPath. > > The PR includes: > Disable JarIndex support in URLClassPath by default. > Add system property jdk.net.URLClassPath.enableJarIndex to re-enable JarIndex > support. This pull request has now been integrated. Changeset: 7700b254 Author:seanwxiang Committer: Hui Shi URL: https://git.openjdk.java.net/jdk/commit/7700b25460b9898060602396fed7bc590ba242b8 Stats: 12 lines in 3 files changed: 8 ins; 1 del; 3 mod 8273401: Disable JarIndex support in URLClassPath Reviewed-by: dfuchs, lancea, alanb, mchung - PR: https://git.openjdk.java.net/jdk/pull/5524
Re: RFR: 8273401: Disable JarIndex support in URLClassPath [v2]
On Thu, 16 Sep 2021 01:29:41 GMT, wxiang
wrote:
>> wxiang has updated the pull request incrementally with one additional commit
>> since the last revision:
>>
>> add isEmpty check
>
> I have changed
>
> ENABLE_JAR_INDEX = p != null ? p.equals("true") : false;
>
> to
>
> ENABLE_JAR_INDEX = p != null ? p.equals("true") || p.isEmpty() : false;
>
>
> Furthemore, in order to maintain consistency in URLClassPath, the property
> name is "jdk.net.URLClassPath.enableJarIndex" .
> @shiyuexw Just a reminder that you need to finalize the CSR.
Thanks a lot. We have finalized the CSR.
-
PR: https://git.openjdk.java.net/jdk/pull/5524
Re: RFR: 8273401: Disable JarIndex support in URLClassPath [v2]
On Thu, 16 Sep 2021 11:08:17 GMT, Alan Bateman wrote: >> wxiang has updated the pull request incrementally with one additional commit >> since the last revision: >> >> add isEmpty check > > src/java.base/share/classes/jdk/internal/loader/URLClassPath.java line 949: > >> 947: return checkResource(name, check, entry); >> 948: >> 949: if (index == null || !ENABLE_JAR_INDEX) > > Is this needed? When ENABLE_JAR_INDEX is false then I assume index will > always be null. I'm only asking is that it would be nice if ENABLE_JAR_INDEX > was checked in one place rather than two. Yes, the check is redundant, and I removed it. - PR: https://git.openjdk.java.net/jdk/pull/5524
Re: RFR: 8273401: Disable JarIndex support in URLClassPath [v3]
> There is a bug for URLClassPath.findResources with JarIndex. > Currently, there was agreement on dropping the support from the > URLClassLoader implementation but it was suggested that it should be disabled > for a release or two before the code is removed. > A system property can be used to re-enable JarIndex support in URLClassPath. > > The PR includes: > Disable JarIndex support in URLClassPath by default. > Add system property jdk.net.URLClassPath.enableJarIndex to re-enable JarIndex > support. wxiang has updated the pull request incrementally with one additional commit since the last revision: Remove redundant check - Changes: - all: https://git.openjdk.java.net/jdk/pull/5524/files - new: https://git.openjdk.java.net/jdk/pull/5524/files/8dd17484..51e169d7 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5524=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5524=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5524.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5524/head:pull/5524 PR: https://git.openjdk.java.net/jdk/pull/5524
Re: RFR: 8273401: Disable JarIndex support in URLClassPath [v2]
On Thu, 16 Sep 2021 09:15:34 GMT, Daniel Fuchs wrote:
>> I can appreciate that and that is a fair point. I guess where I was
>> coming from was that the property is temporary and has to be explicitly
>> specified so ignoring the value seemed worth raising/discussing.
>>
>> Assuming the current check is kept(which we should based on your input), it
>> should probably be changed to ignore case.
>
> Given that there is a precedence in this file:
>
>
> p =
> props.getProperty("jdk.net.URLClassPath.showIgnoredClassPathEntries");
> DEBUG_CP_URL_CHECK = p != null ? p.equals("true") || p.isEmpty() :
> false;
>
>
> I would tend to let the new property follow the same pattern, especially
> since it's temporary.
Yes，I changed the code to follow the same pattern.
-
PR: https://git.openjdk.java.net/jdk/pull/5524
Re: RFR: 8273401: Disable JarIndex support in URLClassPath [v2]
On Thu, 16 Sep 2021 01:29:12 GMT, wxiang
wrote:
>> There is a bug for URLClassPath.findResources with JarIndex.
>> Currently, there was agreement on dropping the support from the
>> URLClassLoader implementation but it was suggested that it should be
>> disabled for a release or two before the code is removed.
>> A system property can be used to re-enable JarIndex support in URLClassPath.
>>
>> The PR includes:
>> Disable JarIndex support in URLClassPath by default.
>> Add system property jdk.net.URLClassPath.enableJarIndex to re-enable
>> JarIndex support.
>
> wxiang has updated the pull request incrementally with one additional commit
> since the last revision:
>
> add isEmpty check
I have changed
ENABLE_JAR_INDEX = p != null ? p.equals("true") : false;
to
ENABLE_JAR_INDEX = p != null ? p.equals("true") || p.isEmpty() : false;
Furthemore, in order to maintain consistency in URLClassPath, the property
name is "jdk.net.URLClassPath.enableJarIndex" .
-
PR: https://git.openjdk.java.net/jdk/pull/5524
Re: RFR: 8273401: Disable JarIndex support in URLClassPath [v2]
> There is a bug for URLClassPath.findResources with JarIndex. > Currently, there was agreement on dropping the support from the > URLClassLoader implementation but it was suggested that it should be disabled > for a release or two before the code is removed. > A system property can be used to re-enable JarIndex support in URLClassPath. > > The PR includes: > Disable JarIndex support in URLClassPath by default. > Add system property jdk.net.URLClassPath.enableJarIndex to re-enable JarIndex > support. wxiang has updated the pull request incrementally with one additional commit since the last revision: add isEmpty check - Changes: - all: https://git.openjdk.java.net/jdk/pull/5524/files - new: https://git.openjdk.java.net/jdk/pull/5524/files/6a6e7b15..8dd17484 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5524=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5524=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5524.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5524/head:pull/5524 PR: https://git.openjdk.java.net/jdk/pull/5524
Re: RFR: 8273401: Remove JarIndex support in URLClassPath [v2]
On Wed, 15 Sep 2021 05:59:13 GMT, Alan Bateman wrote: > > Yes, I will take care of it. Need I create a new PR? > > Up to you, it's okay to continue with this one if you want, we would just > need to change the description here and in JBS. I created a new PR https://github.com/openjdk/jdk/pull/5524 to disable JarIndex and update the description in JBS. - PR: https://git.openjdk.java.net/jdk/pull/5383
RFR: 8273401: Disable JarIndex support in URLClassPath
There is a bug for URLClassPath.findResources with JarIndex. Currently, there was agreement on dropping the support from the URLClassLoader implementation but it was suggested that it should be disabled for a release or two before the code is removed. A system property can be used to re-enable JarIndex support in URLClassPath. The PR includes: Disable JarIndex support in URLClassPath by default. Add system property jdk.net.URLClassPath.enableJarIndex to re-enable JarIndex support. - Commit messages: - 8273401: Disable JarIndex support in URLClassPath Changes: https://git.openjdk.java.net/jdk/pull/5524/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5524=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8273401 Stats: 13 lines in 3 files changed: 8 ins; 1 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/5524.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5524/head:pull/5524 PR: https://git.openjdk.java.net/jdk/pull/5524
Re: RFR: 8273401: Remove JarIndex support in URLClassPath [v2]
On Wed, 8 Sep 2021 06:30:34 GMT, wxiang wrote: >> wxiang has updated the pull request incrementally with one additional commit >> since the last revision: >> >> Some minor changes >> >> Include: >> 1. remove public for INDEX_NAME in JarFile >> 2. recover the definition for INDEX_NAME in JarIndex >> 3. mv the test case JarIndexMergeTest.java into test/jdk/java/util/jar > > Create CSR: https://bugs.openjdk.java.net/browse/JDK-8273473 > @shiyuexw I discussed this issue with other maintainers in this area. There > was agreement on dropping the support from the URLClassLoader implementation > but it was suggested that it should be disabled for a release or two before > the code is removed. A system property can be used to re-enable. I've updated > the CSR to reflect this proposal. Are you okay to continue with this new > proposal? It would mean that we wouldn't remove the tests but instead change > them to run with the system property. Yes, I will take care of it. Need I create a new PR? - PR: https://git.openjdk.java.net/jdk/pull/5383
Re: RFR: 8273401: Remove JarIndex support in URLClassPath [v2]
On Wed, 8 Sep 2021 06:22:38 GMT, wxiang wrote: >> There is a bug for URLClassPath.findResources with JarIndex. >> With some discussions about the bug, the current priority is to remove the >> JAR index support in URLClassPath, >> and don’t need to do anything to the jar tool in the short term, except just >> to move JarIndex to the jdk.jartool module. >> >> The PR includes: >> 1. remove the JarIndex support in URLClassPath >> 2. move JarIndex into jdk.jartool module. > > wxiang has updated the pull request incrementally with one additional commit > since the last revision: > > Some minor changes > > Include: > 1. remove public for INDEX_NAME in JarFile > 2. recover the definition for INDEX_NAME in JarIndex > 3. mv the test case JarIndexMergeTest.java into test/jdk/java/util/jar Create CSR: https://bugs.openjdk.java.net/browse/JDK-8273473 - PR: https://git.openjdk.java.net/jdk/pull/5383
Re: RFR: 8273401: Remove JarIndex support in URLClassPath [v2]
On Tue, 7 Sep 2021 17:39:20 GMT, Sean Mullan wrote:
>> src/java.base/share/classes/java/util/jar/JarVerifier.java line 147:
>>
>>> 145:
>>> 146: if (uname.equals(JarFile.MANIFEST_NAME) ||
>>> 147: uname.equals(JarFile.INDEX_NAME)) {
>>
>> It would be useful if someone from security-libs could comment on this. The
>> interaction between signed JAR and JAR index isn't very clear. The change
>> you have is safe but it might be that we can drop the checking for
>> INDEX.LIST here.
>
> I am thinking this line should not be removed for compatibility with existing
> JARs that have indexes.
still keep the code
-
PR: https://git.openjdk.java.net/jdk/pull/5383
Re: RFR: 8273401: Remove JarIndex support in URLClassPath [v2]
> There is a bug for URLClassPath.findResources with JarIndex. > With some discussions about the bug, the current priority is to remove the > JAR index support in URLClassPath, > and don’t need to do anything to the jar tool in the short term, except just > to move JarIndex to the jdk.jartool module. > > The PR includes: > 1. remove the JarIndex support in URLClassPath > 2. move JarIndex into jdk.jartool module. wxiang has updated the pull request incrementally with one additional commit since the last revision: Some minor changes Include: 1. remove public for INDEX_NAME in JarFile 2. recover the definition for INDEX_NAME in JarIndex 3. mv the test case JarIndexMergeTest.java into test/jdk/java/util/jar - Changes: - all: https://git.openjdk.java.net/jdk/pull/5383/files - new: https://git.openjdk.java.net/jdk/pull/5383/files/7e11c607..cba9047d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5383=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5383=00-01 Stats: 12 lines in 4 files changed: 5 ins; 0 del; 7 mod Patch: https://git.openjdk.java.net/jdk/pull/5383.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5383/head:pull/5383 PR: https://git.openjdk.java.net/jdk/pull/5383
Re: RFR: 8273401: Remove JarIndex support in URLClassPath [v2]
On Tue, 7 Sep 2021 10:39:01 GMT, Lance Andersen wrote: >> src/java.base/share/classes/java/util/jar/JarFile.java line 220: >> >>> 218: * The index file name. >>> 219: */ >>> 220: public static final String INDEX_NAME = "META-INF/INDEX.LIST"; >> >> Adding this as a public field means it becomes part of the API, so it >> shouldn't be public here. > > Agree remove public，but recover the same definition in JarIndex - PR: https://git.openjdk.java.net/jdk/pull/5383
RFR: 8273401: Remove JarIndex support in URLClassPath
There is a bug for URLClassPath.findResources with JarIndex. With some discussions about the bug, the current priority is to remove the JAR index support in URLClassPath, and don’t need to do anything to the jar tool in the short term, except just to move JarIndex to the jdk.jartool module. The PR includes: 1. remove the JarIndex support in URLClassPath 2. move JarIndex into jdk.jartool module. - Commit messages: - 8273401: Remove JarIndex support in URLClassPath Changes: https://git.openjdk.java.net/jdk/pull/5383/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5383=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8273401 Stats: 1117 lines in 21 files changed: 7 ins; 1098 del; 12 mod Patch: https://git.openjdk.java.net/jdk/pull/5383.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5383/head:pull/5383 PR: https://git.openjdk.java.net/jdk/pull/5383
Re: RFR: 6957241: ClassLoader.getResources() returns only 1 instance when using jar indexing
On Tue, 31 Aug 2021 12:11:46 GMT, wxiang
wrote:
> Using jarIndex for Hibench, there is an unexpected behavior with the
> exception "Exception in thread "main"
> org.apache.hadoop.fs.UnsupportedFileSystemException: No FileSystem for scheme
> "hdfs"".
>
> After investigating it, it is related to the usage of ServiceLoader with
> JarIndex.
> The below stack shows the issue with JDK11:
>
> getResource:1016, URLClassPath$JarLoader (jdk.internal.loader)
> getResource:937, URLClassPath$JarLoader (jdk.internal.loader)
> findResource:912, URLClassPath$JarLoader (jdk.internal.loader)
> next:341, URLClassPath$1 (jdk.internal.loader)
> hasMoreElements:351, URLClassPath$1 (jdk.internal.loader)
> hasNext:355, BuiltinClassLoader$1 (jdk.internal.loader)
> hasMoreElements:363, BuiltinClassLoader$1 (jdk.internal.loader)
> next:3032, CompoundEnumeration (java.lang)
> hasMoreElements:3041, CompoundEnumeration (java.lang)
> nextProviderClass:1203, ServiceLoader$LazyClassPathLookupIterator (java.util)
> hasNextService:1221, ServiceLoader$LazyClassPathLookupIterator (java.util)
> hasNext:1265, ServiceLoader$LazyClassPathLookupIterator (java.util)
> hasNext:1300, ServiceLoader$2 (java.util)
> hasNext:1385, ServiceLoader$3 (java.util)
>
> The below API tries to get all the resources with the same name.
>
> public Enumeration findResources(final String name,
> final boolean check)
> ```
> After using JarIndex, URLClassPath.findResources only returns 1 URL.
> It is the same as the description in JDK-6957241.
>
> The issue still exists in JDK18.
>
> Root cause:
>
> public Enumeration findResources(final String name,
> final boolean check) {
> return new Enumeration<>() {
> private int index = 0;
> private URL url = null;
>
> private boolean next() {
> if (url != null) {
> return true;
> } else {
> Loader loader;
> while ((loader = getLoader(index++)) != null) {
> url = loader.findResource(name, check);
> if (url != null) {
> return true;
> }
> }
> return false;
> }
> }
> ...
> };
> }
>
> With the JarIndex, there is only one loader which is corresponding to the jar
> with the index due to the implementation in JarLoader.getResource(final
> String name, boolean check, Set visited).
>
> Loaders corresponding to other jar packages will not appear in this while.
> So it only returns 1 instance.
>
> To solve the issue, I change the implementation "private boolean next()".
> If the loader has index, traverse the index and get all the resource from the
> loader.
I created a new JBS https://bugs.openjdk.java.net/browse/JDK-8273401, and PR:
https://github.com/openjdk/jdk/pull/5383.
The PR is to remove the JarIndex support in URLClassPath and move JarIndex into
the jdk.jartool module.
-
PR: https://git.openjdk.java.net/jdk/pull/5316
Re: RFR: 6957241: ClassLoader.getResources() returns only 1 instance when using jar indexing
On Fri, 3 Sep 2021 10:48:01 GMT, Alan Bateman wrote: > > @AlanBateman Sure, I am interested in it. > > Great! I think there are several parts to this. The removal of the JAR index > support from the URLClassLoader implementation, the `jar i` option, the JAR > file spec, and the jar tool man page. > > It would be good to create a patch for the removal to see if there are any > issues. There will probably need to be some discussion on what to do with the > jar tool. I suspect we will need to keep the code that updates the index when > updating a JAR file that has an existing index, this means keeping JarIndex > and maybe moving it to the jdk.jartool module. We can change `jar i` to print > a warning when the tool is called to generate an index. Don't worry about the > JAR spec and man page for now. I will first create the patch to remove JAR index support from the URLClassLoader implementation, the `jar i` option. - PR: https://git.openjdk.java.net/jdk/pull/5316
Re: RFR: 6957241: ClassLoader.getResources() returns only 1 instance when using jar indexing
On Thu, 2 Sep 2021 11:43:46 GMT, Alan Bateman wrote:
>> Using jarIndex for Hibench, there is an unexpected behavior with the
>> exception "Exception in thread "main"
>> org.apache.hadoop.fs.UnsupportedFileSystemException: No FileSystem for
>> scheme "hdfs"".
>>
>> After investigating it, it is related to the usage of ServiceLoader with
>> JarIndex.
>> The below stack shows the issue with JDK11:
>>
>> getResource:1016, URLClassPath$JarLoader (jdk.internal.loader)
>> getResource:937, URLClassPath$JarLoader (jdk.internal.loader)
>> findResource:912, URLClassPath$JarLoader (jdk.internal.loader)
>> next:341, URLClassPath$1 (jdk.internal.loader)
>> hasMoreElements:351, URLClassPath$1 (jdk.internal.loader)
>> hasNext:355, BuiltinClassLoader$1 (jdk.internal.loader)
>> hasMoreElements:363, BuiltinClassLoader$1 (jdk.internal.loader)
>> next:3032, CompoundEnumeration (java.lang)
>> hasMoreElements:3041, CompoundEnumeration (java.lang)
>> nextProviderClass:1203, ServiceLoader$LazyClassPathLookupIterator (java.util)
>> hasNextService:1221, ServiceLoader$LazyClassPathLookupIterator (java.util)
>> hasNext:1265, ServiceLoader$LazyClassPathLookupIterator (java.util)
>> hasNext:1300, ServiceLoader$2 (java.util)
>> hasNext:1385, ServiceLoader$3 (java.util)
>>
>> The below API tries to get all the resources with the same name.
>>
>> public Enumeration findResources(final String name,
>> final boolean check)
>> ```
>> After using JarIndex, URLClassPath.findResources only returns 1 URL.
>> It is the same as the description in JDK-6957241.
>>
>> The issue still exists in JDK18.
>>
>> Root cause:
>>
>> public Enumeration findResources(final String name,
>> final boolean check) {
>> return new Enumeration<>() {
>> private int index = 0;
>> private URL url = null;
>>
>> private boolean next() {
>> if (url != null) {
>> return true;
>> } else {
>> Loader loader;
>> while ((loader = getLoader(index++)) != null) {
>> url = loader.findResource(name, check);
>> if (url != null) {
>> return true;
>> }
>> }
>> return false;
>> }
>> }
>> ...
>> };
>> }
>>
>> With the JarIndex, there is only one loader which is corresponding to the
>> jar with the index due to the implementation in JarLoader.getResource(final
>> String name, boolean check, Set visited).
>>
>> Loaders corresponding to other jar packages will not appear in this while.
>> So it only returns 1 instance.
>>
>> To solve the issue, I change the implementation "private boolean next()".
>> If the loader has index, traverse the index and get all the resource from
>> the loader.
>
> @wxiang I think there is at least some support for removing the JAR indexing
> support rather than trying to fix findResources. The issue of what to do with
> the legacy JAR index mechanism came up during JDK 9 in the context of modular
> JARs and also Multi-Release JARs but it was too much to take on at the time.
> Would you be interested in working out the changes to remove it?
@AlanBateman Sure, I am interested in it.
-
PR: https://git.openjdk.java.net/jdk/pull/5316
RFR: 6957241: ClassLoader.getResources() returns only 1 instance when using jar indexing
Using jarIndex for Hibench, there is an unexpected behavior with the exception
"Exception in thread "main"
org.apache.hadoop.fs.UnsupportedFileSystemException: No FileSystem for scheme
"hdfs"".
After investigating it, it is related to the usage of ServiceLoader with
JarIndex.
The below stack shows the issue with JDK11:
getResource:1016, URLClassPath$JarLoader (jdk.internal.loader)
getResource:937, URLClassPath$JarLoader (jdk.internal.loader)
findResource:912, URLClassPath$JarLoader (jdk.internal.loader)
next:341, URLClassPath$1 (jdk.internal.loader)
hasMoreElements:351, URLClassPath$1 (jdk.internal.loader)
hasNext:355, BuiltinClassLoader$1 (jdk.internal.loader)
hasMoreElements:363, BuiltinClassLoader$1 (jdk.internal.loader)
next:3032, CompoundEnumeration (java.lang)
hasMoreElements:3041, CompoundEnumeration (java.lang)
nextProviderClass:1203, ServiceLoader$LazyClassPathLookupIterator (java.util)
hasNextService:1221, ServiceLoader$LazyClassPathLookupIterator (java.util)
hasNext:1265, ServiceLoader$LazyClassPathLookupIterator (java.util)
hasNext:1300, ServiceLoader$2 (java.util)
hasNext:1385, ServiceLoader$3 (java.util)
The below API tries to get all the resources with the same name.
public Enumeration findResources(final String name,
final boolean check)
```
After using JarIndex, URLClassPath.findResources only returns 1 URL.
It is the same as the description in JDK-6957241.
The issue still exists in JDK18.
Root cause:
public Enumeration findResources(final String name,
final boolean check) {
return new Enumeration<>() {
private int index = 0;
private URL url = null;
private boolean next() {
if (url != null) {
return true;
} else {
Loader loader;
while ((loader = getLoader(index++)) != null) {
url = loader.findResource(name, check);
if (url != null) {
return true;
}
}
return false;
}
}
...
};
}
With the JarIndex, there is only one loader which is corresponding to the jar
with the index due to the implementation in JarLoader.getResource(final String
name, boolean check, Set visited).
Loaders corresponding to other jar packages will not appear in this while.
So it only returns 1 instance.
To solve the issue, I change the implementation "private boolean next()".
If the loader has index, traverse the index and get all the resource from the
loader.
-
Commit messages:
- solve Whitespace error
- 6957241: ClassLoader.getResources() returns only 1 instance when using jar
indexing
Changes: https://git.openjdk.java.net/jdk/pull/5316/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5316=00
Issue: https://bugs.openjdk.java.net/browse/JDK-6957241
Stats: 769 lines in 8 files changed: 763 ins; 0 del; 6 mod
Patch: https://git.openjdk.java.net/jdk/pull/5316.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/5316/head:pull/5316
PR: https://git.openjdk.java.net/jdk/pull/5316
