RFR: 8264048: Fix caching in Jar URL connections when an entry is missing

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: 8264048: Fix caching in Jar URL connections when an entry is missing

Aleksei Efimov
Current fix tries to tackle an issue with URL connection referencing non-existing Jar file entries:
If an entry that doesn't exist is specified in an URL connection the underlying Jar file is still cached even if an exception is thrown after that. Such behavior prevents the caller, for instance, a `URLClassLoader`, from closing a Jar file.

The proposed fix checks if entry exists before caching a Jar file (only for cases with enabled caching):
- If entry exists - jar file is cached if it wasn't cached before
- If entry doesn't exist and jar file wasn't cached before - jar file is closed and exception is thrown
- If entry doesn't exist and jar file was cached before - jar file is kept cached and exception is thrown


The following tests have been used to verify the fix:
- New regression tests
- ``:jdk_core:`` tests
- `api/java_util`,`api/java_net` JCK tests

-------------

Commit messages:
 - Merge remote-tracking branch 'origin' into JDK-8264048
 - 8264048: Fix caching in Jar URL connections when an entry is missing

Changes: https://git.openjdk.java.net/jdk/pull/3263/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3263&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8264048
  Stats: 551 lines in 7 files changed: 525 ins; 2 del; 24 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3263.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3263/head:pull/3263

PR: https://git.openjdk.java.net/jdk/pull/3263
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264048: Fix caching in Jar URL connections when an entry is missing

Daniel Fuchs-2
On Tue, 30 Mar 2021 11:30:48 GMT, Aleksei Efimov <[hidden email]> wrote:

> Current fix tries to tackle an issue with URL connection referencing non-existing Jar file entries:
> If an entry that doesn't exist is specified in an URL connection the underlying Jar file is still cached even if an exception is thrown after that. Such behavior prevents the caller, for instance, a `URLClassLoader`, from closing a Jar file.
>
> The proposed fix checks if entry exists before caching a Jar file (only for cases with enabled caching):
> - If entry exists - jar file is cached if it wasn't cached before
> - If entry doesn't exist and jar file wasn't cached before - jar file is closed and exception is thrown
> - If entry doesn't exist and jar file was cached before - jar file is kept cached and exception is thrown
>
>
> The following tests have been used to verify the fix:
> - New regression tests
> - ``:jdk_core:`` tests
> - `api/java_util`,`api/java_net` JCK tests

Hi Aleksei, thanks for putting this together.

`test/jdk/sun/misc/URLClassPath/RemoveJar.java` seems to be an older version of `test/jdk/java/net/URLClassLoader/RemoveJar.java`. The two tests are almost identical - so `test/jdk/sun/misc/URLClassPath/RemoveJar.java` can probably be removed from the PR.

Otherwise the proposed changes look good to me.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3263
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264048: Fix caching in Jar URL connections when an entry is missing [v2]

Aleksei Efimov
In reply to this post by Aleksei Efimov
> Current fix tries to tackle an issue with URL connection referencing non-existing Jar file entries:
> If an entry that doesn't exist is specified in an URL connection the underlying Jar file is still cached even if an exception is thrown after that. Such behavior prevents the caller, for instance, a `URLClassLoader`, from closing a Jar file.
>
> The proposed fix checks if entry exists before caching a Jar file (only for cases with enabled caching):
> - If entry exists - jar file is cached if it wasn't cached before
> - If entry doesn't exist and jar file wasn't cached before - jar file is closed and exception is thrown
> - If entry doesn't exist and jar file was cached before - jar file is kept cached and exception is thrown
>
>
> The following tests have been used to verify the fix:
> - New regression tests
> - ``:jdk_core:`` tests
> - `api/java_util`,`api/java_net` JCK tests

Aleksei Efimov has updated the pull request incrementally with one additional commit since the last revision:

  JDK-8264048: Remove old version of RemoveJar test

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3263/files
  - new: https://git.openjdk.java.net/jdk/pull/3263/files/6aeb3333..2f0fa527

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3263&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3263&range=00-01

  Stats: 179 lines in 1 file changed: 0 ins; 179 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3263.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3263/head:pull/3263

PR: https://git.openjdk.java.net/jdk/pull/3263
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264048: Fix caching in Jar URL connections when an entry is missing

Aleksei Efimov
In reply to this post by Daniel Fuchs-2
On Wed, 31 Mar 2021 11:24:02 GMT, Daniel Fuchs <[hidden email]> wrote:

>> Current fix tries to tackle an issue with URL connection referencing non-existing Jar file entries:
>> If an entry that doesn't exist is specified in an URL connection the underlying Jar file is still cached even if an exception is thrown after that. Such behavior prevents the caller, for instance, a `URLClassLoader`, from closing a Jar file.
>>
>> The proposed fix checks if entry exists before caching a Jar file (only for cases with enabled caching):
>> - If entry exists - jar file is cached if it wasn't cached before
>> - If entry doesn't exist and jar file wasn't cached before - jar file is closed and exception is thrown
>> - If entry doesn't exist and jar file was cached before - jar file is kept cached and exception is thrown
>>
>>
>> The following tests have been used to verify the fix:
>> - New regression tests
>> - ``:jdk_core:`` tests
>> - `api/java_util`,`api/java_net` JCK tests
>
> Hi Aleksei, thanks for putting this together.
>
> `test/jdk/sun/misc/URLClassPath/RemoveJar.java` seems to be an older version of `test/jdk/java/net/URLClassLoader/RemoveJar.java`. The two tests are almost identical - so `test/jdk/sun/misc/URLClassPath/RemoveJar.java` can probably be removed from the PR.
>
> Otherwise the proposed changes look good to me.

Thanks for the review, Daniel. It is correct that `test/jdk/sun/misc/URLClassPath/RemoveJar.java` is an old version. It is removed now.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3263
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264048: Fix caching in Jar URL connections when an entry is missing [v2]

Brent Christian
In reply to this post by Aleksei Efimov
On Wed, 31 Mar 2021 12:47:50 GMT, Aleksei Efimov <[hidden email]> wrote:

>> Current fix tries to tackle an issue with URL connection referencing non-existing Jar file entries:
>> If an entry that doesn't exist is specified in an URL connection the underlying Jar file is still cached even if an exception is thrown after that. Such behavior prevents the caller, for instance, a `URLClassLoader`, from closing a Jar file.
>>
>> The proposed fix checks if entry exists before caching a Jar file (only for cases with enabled caching):
>> - If entry exists - jar file is cached if it wasn't cached before
>> - If entry doesn't exist and jar file wasn't cached before - jar file is closed and exception is thrown
>> - If entry doesn't exist and jar file was cached before - jar file is kept cached and exception is thrown
>>
>>
>> The following tests have been used to verify the fix:
>> - New regression tests
>> - ``:jdk_core:`` tests
>> - `api/java_util`,`api/java_net` JCK tests
>
> Aleksei Efimov has updated the pull request incrementally with one additional commit since the last revision:
>
>   JDK-8264048: Remove old version of RemoveJar test

Hi, Aleksei.  The change looks good.

-------------

Marked as reviewed by bchristi (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/3263
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264048: Fix caching in Jar URL connections when an entry is missing [v2]

Daniel Fuchs-2
In reply to this post by Aleksei Efimov
On Wed, 31 Mar 2021 12:47:50 GMT, Aleksei Efimov <[hidden email]> wrote:

>> Current fix tries to tackle an issue with URL connection referencing non-existing Jar file entries:
>> If an entry that doesn't exist is specified in an URL connection the underlying Jar file is still cached even if an exception is thrown after that. Such behavior prevents the caller, for instance, a `URLClassLoader`, from closing a Jar file.
>>
>> The proposed fix checks if entry exists before caching a Jar file (only for cases with enabled caching):
>> - If entry exists - jar file is cached if it wasn't cached before
>> - If entry doesn't exist and jar file wasn't cached before - jar file is closed and exception is thrown
>> - If entry doesn't exist and jar file was cached before - jar file is kept cached and exception is thrown
>>
>>
>> The following tests have been used to verify the fix:
>> - New regression tests
>> - ``:jdk_core:`` tests
>> - `api/java_util`,`api/java_net` JCK tests
>
> Aleksei Efimov has updated the pull request incrementally with one additional commit since the last revision:
>
>   JDK-8264048: Remove old version of RemoveJar test

Marked as reviewed by dfuchs (Reviewer).

-------------

PR: https://git.openjdk.java.net/jdk/pull/3263
Reply | Threaded
Open this post in threaded view
|

Integrated: 8264048: Fix caching in Jar URL connections when an entry is missing

Aleksei Efimov
In reply to this post by Aleksei Efimov
On Tue, 30 Mar 2021 11:30:48 GMT, Aleksei Efimov <[hidden email]> wrote:

> Current fix tries to tackle an issue with URL connection referencing non-existing Jar file entries:
> If an entry that doesn't exist is specified in an URL connection the underlying Jar file is still cached even if an exception is thrown after that. Such behavior prevents the caller, for instance, a `URLClassLoader`, from closing a Jar file.
>
> The proposed fix checks if entry exists before caching a Jar file (only for cases with enabled caching):
> - If entry exists - jar file is cached if it wasn't cached before
> - If entry doesn't exist and jar file wasn't cached before - jar file is closed and exception is thrown
> - If entry doesn't exist and jar file was cached before - jar file is kept cached and exception is thrown
>
>
> The following tests have been used to verify the fix:
> - New regression tests
> - ``:jdk_core:`` tests
> - `api/java_util`,`api/java_net` JCK tests

This pull request has now been integrated.

Changeset: a611c462
Author:    Aleksei Efimov <[hidden email]>
URL:       https://git.openjdk.java.net/jdk/commit/a611c462
Stats:     372 lines in 6 files changed: 346 ins; 2 del; 24 mod

8264048: Fix caching in Jar URL connections when an entry is missing

Co-authored-by: Daniel Fuchs <[hidden email]>
Reviewed-by: bchristi, dfuchs

-------------

PR: https://git.openjdk.java.net/jdk/pull/3263