Re: RFR: 8275731: CDS archived enums objects are recreated at runtime [v3]

2021-12-10 Thread Ioi Lam
> **Background:**
> 
> In the Java Language, Enums can be tested for equality, so the constants in 
> an Enum type must be unique. Javac compiles an enum declaration like this:
> 
> 
> public enum Day {  SUNDAY, MONDAY ... } 
> 
> 
> to
> 
> 
> public class Day extends java.lang.Enum {
> public static final SUNDAY = new Day("SUNDAY");
> public static final MONDAY = new Day("MONDAY"); ...
> }
> 
> 
> With CDS archived heap objects, `Day::` is executed twice: once 
> during `java -Xshare:dump`, and once during normal JVM execution. If the 
> archived heap objects references one of the Enum constants created at dump 
> time, we will violate the uniqueness requirements of the Enum constants at 
> runtime. See the test case in the description of 
> [JDK-8275731](https://bugs.openjdk.java.net/browse/JDK-8275731)
> 
> **Fix:**
> 
> During -Xshare:dump, if we discovered that an Enum constant of type X is 
> archived, we archive all constants of type X. At Runtime, type X will skip 
> the normal execution of `X::`. Instead, we run 
> `HeapShared::initialize_enum_klass()` to retrieve all the constants of X that 
> were saved at dump time.
> 
> This is safe as we know that `X::` has no observable side effect -- 
> it only creates the constants of type X, as well as the synthetic value 
> `X::$VALUES`, which cannot be observed until X is fully initialized.
> 
> **Verification:**
> 
> To avoid future problems, I added a new tool, CDSHeapVerifier, to look for 
> similar problems where the archived heap objects reference a static field 
> that may be recreated at runtime. There are some manual steps involved, but I 
> analyzed the potential problems found by the tool are they are all safe 
> (after the current bug is fixed). See cdsHeapVerifier.cpp for gory details. 
> An example trace of this tool can be found at 
> https://bugs.openjdk.java.net/secure/attachment/97242/enum_warning.txt
> 
> **Testing:**
> 
> Passed Oracle CI tiers 1-4. WIll run tier 5 as well.

Ioi Lam has updated the pull request with a new target base due to a merge or a 
rebase. The incremental webrev excludes the unrelated changes brought in by the 
merge/rebase. The pull request contains four additional commits since the last 
revision:

 - Merge branch 'master' into 8275731-heapshared-enum
 - added exclusions needed by "java -Xshare:dump -ea -esa"
 - Comments from @calvinccheung off-line
 - 8275731: CDS archived enums objects are recreated at runtime

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6653/files
  - new: https://git.openjdk.java.net/jdk/pull/6653/files/df0d3f88..6e160057

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=6653=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6653=01-02

  Stats: 24204 lines in 951 files changed: 16523 ins; 3176 del; 4505 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6653.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6653/head:pull/6653

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


Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v20]

2021-12-10 Thread Kevin Rushforth
On Tue, 7 Dec 2021 13:27:44 GMT, Alan Bateman  wrote:

>> @jddarcy hi Joe, what's the next step with the CSR now? 
>> https://bugs.openjdk.java.net/browse/JDK-8277755
>> thanks
>
>> @jddarcy hi Joe, what's the next step with the CSR now? 
>> https://bugs.openjdk.java.net/browse/JDK-8277755
>> thanks
> 
> For the CSR then you need to press "Finalize".

@AlanBateman will need to review your request.

-

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


Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v25]

2021-12-10 Thread Andrew Leonard
> Add a new --source-date  (epoch seconds) option to jar and jmod to 
> allow specification of time to use for created/updated jar/jmod entries. This 
> then allows the ability to make the content deterministic.
> 
> Signed-off-by: Andrew Leonard 

Andrew Leonard has updated the pull request incrementally with one additional 
commit since the last revision:

  8276766: Enable jar and jmod to produce deterministic timestamped content
  
  Signed-off-by: Andrew Leonard 

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6481/files
  - new: https://git.openjdk.java.net/jdk/pull/6481/files/0a69d014..f99fc418

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=6481=24
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6481=23-24

  Stats: 96 lines in 1 file changed: 4 ins; 5 del; 87 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6481.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6481/head:pull/6481

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


Re: RFR: 8278065: Refactor subclassAudits to use ClassValue [v3]

2021-12-10 Thread Roman Kennke
> As a follow-up to #6375, this change refactors 
> java.io.ObjectInputStream.Caches#subclassAudits and 
> java.io.ObjectOutputStream.Caches#subclassAudits to use ClassValue instead of 
> SoftReference, similar to what we did in #6375 for 
> java.io.ObjectStreamClass.Caches#localDescs. Then we can now also remove the 
> common machinery java.io.ObjectStreamClass#processQueue and 
> java.io.ObjectStreamClass.WeakClassKey.
> 
> Testing:
>  - [x] tier1
>  - [x] tier2
>  - [ ] tier3

Roman Kennke has updated the pull request with a new target base due to a merge 
or a rebase. The pull request now contains ten commits:

 - Merge branch 'master' into JDK-8278065
 - 8278065: Refactor subclassAudits to use ClassValue
 - Remove unused EntryFuture inner class from ObjectSteamClass
 - Merge remote-tracking branch 'jdk-plevart/JDK-8277072-peter' into JDK-8277072
 - Use ClassValue to solve JDK-8277072
 - Use ForceGC instead of System.gc()
 - Convert test to testng
 - Fix indentation of new testcase
 - 8277072: ObjectStreamClass caches keep ClassLoaders alive

-

Changes: https://git.openjdk.java.net/jdk/pull/6637/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6637=02
  Stats: 105 lines in 3 files changed: 2 ins; 87 del; 16 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6637.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6637/head:pull/6637

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


Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v20]

2021-12-10 Thread Andrew Leonard
On Fri, 10 Dec 2021 17:20:40 GMT, Kevin Rushforth  wrote:

>> @LanceAndersen let me know if mach5 looks good please, then I think we're 
>> good to go..
>
> @andrew-m-leonard if this enhancement is now intended to go into JDK 19, then 
> you can simply integrate it into jdk mainline when it is ready. In that case, 
> the fix version of the 
> [CSR](https://bugs.openjdk.java.net/browse/JDK-8277755) should be adjusted to 
> 19 to match.
> 
> If, however, you would still like to get it into JDK 18, you will need to 
> make a late enhancement request per [JEP 3](https://openjdk.java.net/jeps/3), 
> and then recreate this PR against the jdk18 stabilization repo.

@kevinrushforth fyi: https://bugs.openjdk.java.net/issues/?filter=33407

-

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


Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v20]

2021-12-10 Thread Andrew Leonard
On Fri, 10 Dec 2021 19:16:49 GMT, John Neffenger  wrote:

>>> Thanks, CSR now Finalized
>> 
>> Just a minor note: the CSR uses the adjective "extended" in three places for 
>> the DOS date and time field, but that field is actually a part of the 
>> original ZIP specification and not in an extended field. This implementation 
>> make a point never to touch the "Extended Timestamp Extra Field" defined in 
>> the 1997 [Info-ZIP Application Note 970311][1].
>> 
>> Maybe the confusion was from the required ISO 8601 extended format (rather 
>> than basic).
>> 
>> [1]: 
>> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/zip/ZipEntry.html#setExtra(byte%5B%5D)
>
>> @jgneff John, I know you have an interest in this, what is your urgency for 
>> this support? jdk-18 or 19 ?
> 
> It's not urgent. I'm just being impatient. 
> 
> If this pull request is integrated only into JDK 19, JavaFX won't be able to 
> support reproducible builds until OpenJFX 20 in March 2023. Java projects in 
> general are late to the reproducible builds party. Debian, for example, 
> builds 31,571 packages and [less than three percent fail][1] to build in a 
> reproducible manner. Those failing packages include OpenJDK and OpenJFX. 
> Debian plans eventually to make [reproducibility a requirement][2], and other 
> distributions may follow.
> 
> The only true urgency, of course, is to provide Java project owners better 
> tools to detect the next supply chain attack on the packages they distribute.
> 
> [1]: 
> https://tests.reproducible-builds.org/debian/bookworm/index_suite_amd64_stats.html
> [2]: https://www.debian.org/doc/debian-policy/ch-source.html#reproducibility

@jgneff thanks John, i'm going to raise the JEP 3 request and see where I get, 
as I concur with your statement:

> The only true urgency, of course, is to provide Java project owners better 
> tools to detect the next supply chain attack on the packages they distribute.

The risk is minimal, also given the extensive testing we have done.

-

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


Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v20]

2021-12-10 Thread Lance Andersen
On Tue, 7 Dec 2021 19:25:05 GMT, Lance Andersen  wrote:

>> Andrew Leonard has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   8276766: Enable jar and jmod to produce deterministic timestamped content
>>   
>>   Signed-off-by: Andrew Leonard 
>
> Looks like the CSR has been approved.  I have a mach5 run that should 
> hopefully finish sooner rather than later and if that remains happy, I will 
> approve the PR

> @LanceAndersen let me know if mach5 looks good please, then I think we're 
> good to go..

As soon as  the Mach5 finishes, I will let you know

-

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


Re: RFR: 8271079: JavaFileObject#toUri and multi-release jars [v3]

2021-12-10 Thread Lance Andersen
On Fri, 10 Dec 2021 08:16:46 GMT, Christian Stein  wrote:

>> Prior to this PR, `toUri()` of class `ZipPath` in module `jdk.zipfs` and 
>> class `PathFileObject` in module `jdk.compiler` were always composed by base 
>> path names. Even for versioned entries of a multi-release JAR file.
>> 
>> Now, a `URI` for an entry is composed of its real path names using an 
>> existing lookup function in the associated zip file system object.
>> 
>> This PR also removes a superseded work around for 
>> [JDK-8134451](https://bugs.openjdk.java.net/browse/JDK-8134451).
>> 
>> Fixes https://bugs.openjdk.java.net/browse/JDK-8271079
>
> Christian Stein has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Turn `getRealPath()` into a no-arg helper method

The changes look good.

-

Marked as reviewed by lancea (Reviewer).

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


Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v20]

2021-12-10 Thread John Neffenger
On Tue, 7 Dec 2021 19:06:17 GMT, John Neffenger  wrote:

>> Thanks, CSR now Finalized
>
>> Thanks, CSR now Finalized
> 
> Just a minor note: the CSR uses the adjective "extended" in three places for 
> the DOS date and time field, but that field is actually a part of the 
> original ZIP specification and not in an extended field. This implementation 
> make a point never to touch the "Extended Timestamp Extra Field" defined in 
> the 1997 [Info-ZIP Application Note 970311][1].
> 
> Maybe the confusion was from the required ISO 8601 extended format (rather 
> than basic).
> 
> [1]: 
> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/zip/ZipEntry.html#setExtra(byte%5B%5D)

> @jgneff John, I know you have an interest in this, what is your urgency for 
> this support? jdk-18 or 19 ?

It's not urgent. I'm just being impatient. 

If this pull request is integrated only into JDK 19, JavaFX won't be able to 
support reproducible builds until OpenJFX 20 in March 2023. Java projects in 
general are late to the reproducible builds party. Debian, for example, builds 
31,571 packages and [less than three percent fail][1] to build in a 
reproducible manner. Those failing packages include OpenJDK and OpenJFX. Debian 
plans eventually to make [reproducibility a requirement][2], and other 
distributions may follow.

The only true urgency, of course, is to provide Java project owners better 
tools to detect the next supply chain attack on the packages they distribute.

[1]: 
https://tests.reproducible-builds.org/debian/bookworm/index_suite_amd64_stats.html
[2]: https://www.debian.org/doc/debian-policy/ch-source.html#reproducibility

-

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


Re: RFR: 8278065: Refactor subclassAudits to use ClassValue [v2]

2021-12-10 Thread Erik Joelsson
On Fri, 10 Dec 2021 16:31:45 GMT, Roman Kennke  wrote:

>> As a follow-up to #6375, this change refactors 
>> java.io.ObjectInputStream.Caches#subclassAudits and 
>> java.io.ObjectOutputStream.Caches#subclassAudits to use ClassValue instead 
>> of SoftReference, similar to what we did in #6375 for 
>> java.io.ObjectStreamClass.Caches#localDescs. Then we can now also remove the 
>> common machinery java.io.ObjectStreamClass#processQueue and 
>> java.io.ObjectStreamClass.WeakClassKey.
>> 
>> Testing:
>>  - [x] tier1
>>  - [x] tier2
>>  - [ ] tier3
>
> Roman Kennke has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains eight commits:
> 
>  - 8278065: Refactor subclassAudits to use ClassValue
>  - Remove unused EntryFuture inner class from ObjectSteamClass
>  - Merge remote-tracking branch 'jdk-plevart/JDK-8277072-peter' into 
> JDK-8277072
>  - Use ClassValue to solve JDK-8277072
>  - Use ForceGC instead of System.gc()
>  - Convert test to testng
>  - Fix indentation of new testcase
>  - 8277072: ObjectStreamClass caches keep ClassLoaders alive

> Skara is failing to run jcheck on this PR. I think this is ultimately a bug 
> in Skara, that gets tickled by your source branch being quite far behind 
> jdk:master, in combination with 
> [62a7f5d](https://github.com/openjdk/jdk/commit/62a7f5d3236ab2248518a475b1d8b71cb4bf1313)
>  recently going in. If you merge jdk:master into your branch, I believe this 
> will resolve itself for now.

I've investigated further and filed 
https://bugs.openjdk.java.net/browse/SKARA-1281. Once fixed you would get an 
error message in this PR telling you to rebase your source branch.

-

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


Integrated: Merge jdk18

2021-12-10 Thread Jesper Wilhelmsson
On Fri, 10 Dec 2021 17:51:31 GMT, Jesper Wilhelmsson  
wrote:

> Forwardport JDK 18 -> JDK 19

This pull request has now been integrated.

Changeset: 61736f81
Author:Jesper Wilhelmsson 
URL:   
https://git.openjdk.java.net/jdk/commit/61736f81fb4a20375c83d59e2b37a00aafb11107
Stats: 1142 lines in 30 files changed: 688 ins; 155 del; 299 mod

Merge

-

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


Re: RFR: 8278065: Refactor subclassAudits to use ClassValue [v2]

2021-12-10 Thread Erik Joelsson
On Fri, 10 Dec 2021 16:31:45 GMT, Roman Kennke  wrote:

>> As a follow-up to #6375, this change refactors 
>> java.io.ObjectInputStream.Caches#subclassAudits and 
>> java.io.ObjectOutputStream.Caches#subclassAudits to use ClassValue instead 
>> of SoftReference, similar to what we did in #6375 for 
>> java.io.ObjectStreamClass.Caches#localDescs. Then we can now also remove the 
>> common machinery java.io.ObjectStreamClass#processQueue and 
>> java.io.ObjectStreamClass.WeakClassKey.
>> 
>> Testing:
>>  - [x] tier1
>>  - [x] tier2
>>  - [ ] tier3
>
> Roman Kennke has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains eight commits:
> 
>  - 8278065: Refactor subclassAudits to use ClassValue
>  - Remove unused EntryFuture inner class from ObjectSteamClass
>  - Merge remote-tracking branch 'jdk-plevart/JDK-8277072-peter' into 
> JDK-8277072
>  - Use ClassValue to solve JDK-8277072
>  - Use ForceGC instead of System.gc()
>  - Convert test to testng
>  - Fix indentation of new testcase
>  - 8277072: ObjectStreamClass caches keep ClassLoaders alive

Skara is failing to run jcheck on this PR. I think this is ultimately a bug in 
Skara, that gets tickled by your source branch being quite far behind 
jdk:master, in combination with 62a7f5d3236ab2248518a475b1d8b71cb4bf1313 
recently going in. If you merge jdk:master into your branch, I believe this 
will resolve itself for now.

-

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


RFR: Merge jdk18

2021-12-10 Thread Jesper Wilhelmsson
Forwardport JDK 18 -> JDK 19

-

Commit messages:
 - Merge
 - 8277621: ARM32: multiple fastdebug failures with "bad AD file" after 
JDK-8276162
 - 8278538: Test langtools/jdk/javadoc/tool/CheckManPageOptions.java fails 
after the manpage was updated
 - 8273179: Update nroff pages in JDK 18 before RC

The merge commit only contains trivial merges, so no merge-specific webrevs 
have been generated.

Changes: https://git.openjdk.java.net/jdk/pull/6802/files
  Stats: 1142 lines in 30 files changed: 688 ins; 155 del; 299 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6802.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6802/head:pull/6802

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


Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v20]

2021-12-10 Thread Andrew Leonard
On Tue, 7 Dec 2021 19:06:17 GMT, John Neffenger  wrote:

>> Thanks, CSR now Finalized
>
>> Thanks, CSR now Finalized
> 
> Just a minor note: the CSR uses the adjective "extended" in three places for 
> the DOS date and time field, but that field is actually a part of the 
> original ZIP specification and not in an extended field. This implementation 
> make a point never to touch the "Extended Timestamp Extra Field" defined in 
> the 1997 [Info-ZIP Application Note 970311][1].
> 
> Maybe the confusion was from the required ISO 8601 extended format (rather 
> than basic).
> 
> [1]: 
> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/zip/ZipEntry.html#setExtra(byte%5B%5D)

@jgneff John, I know you have an interest in this, what is your urgency for 
this support? jdk-18 or 19 ?

-

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


Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v20]

2021-12-10 Thread Andrew Leonard
On Fri, 10 Dec 2021 17:20:40 GMT, Kevin Rushforth  wrote:

>> @LanceAndersen let me know if mach5 looks good please, then I think we're 
>> good to go..
>
> @andrew-m-leonard if this enhancement is now intended to go into JDK 19, then 
> you can simply integrate it into jdk mainline when it is ready. In that case, 
> the fix version of the 
> [CSR](https://bugs.openjdk.java.net/browse/JDK-8277755) should be adjusted to 
> 19 to match.
> 
> If, however, you would still like to get it into JDK 18, you will need to 
> make a late enhancement request per [JEP 3](https://openjdk.java.net/jeps/3), 
> and then recreate this PR against the jdk18 stabilization repo.

@kevinrushforth thanks Kevin, I had totally missed that RDP1 was yesterday! 
i'll have some thought on what's best, cheers

-

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


Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v20]

2021-12-10 Thread Kevin Rushforth
On Fri, 10 Dec 2021 17:01:41 GMT, Andrew Leonard  wrote:

>> Looks like the CSR has been approved.  I have a mach5 run that should 
>> hopefully finish sooner rather than later and if that remains happy, I will 
>> approve the PR
>
> @LanceAndersen let me know if mach5 looks good please, then I think we're 
> good to go..

@andrew-m-leonard if this enhancement is now intended to go into JDK 19, then 
you can simply integrate it into jdk mainline when it is ready. In that case, 
the fix version of the [CSR](https://bugs.openjdk.java.net/browse/JDK-8277755) 
should be adjusted to 19 to match.

If, however, you would still like to get it into JDK 18, you will need to make 
a late enhancement request per [JEP 3](https://openjdk.java.net/jeps/3), and 
then recreate this PR against the jdk18 stabilization repo.

-

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


Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v20]

2021-12-10 Thread Andrew Leonard
On Tue, 7 Dec 2021 19:25:05 GMT, Lance Andersen  wrote:

>> Andrew Leonard has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   8276766: Enable jar and jmod to produce deterministic timestamped content
>>   
>>   Signed-off-by: Andrew Leonard 
>
> Looks like the CSR has been approved.  I have a mach5 run that should 
> hopefully finish sooner rather than later and if that remains happy, I will 
> approve the PR

@LanceAndersen let me know if mach5 looks good please, then I think we're good 
to go..

-

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


Re: RFR: 8278065: Refactor subclassAudits to use ClassValue [v2]

2021-12-10 Thread Roman Kennke
> As a follow-up to #6375, this change refactors 
> java.io.ObjectInputStream.Caches#subclassAudits and 
> java.io.ObjectOutputStream.Caches#subclassAudits to use ClassValue instead of 
> SoftReference, similar to what we did in #6375 for 
> java.io.ObjectStreamClass.Caches#localDescs. Then we can now also remove the 
> common machinery java.io.ObjectStreamClass#processQueue and 
> java.io.ObjectStreamClass.WeakClassKey.
> 
> Testing:
>  - [x] tier1
>  - [x] tier2
>  - [ ] tier3

Roman Kennke has updated the pull request with a new target base due to a merge 
or a rebase. The pull request now contains eight commits:

 - 8278065: Refactor subclassAudits to use ClassValue
 - Remove unused EntryFuture inner class from ObjectSteamClass
 - Merge remote-tracking branch 'jdk-plevart/JDK-8277072-peter' into JDK-8277072
 - Use ClassValue to solve JDK-8277072
 - Use ForceGC instead of System.gc()
 - Convert test to testng
 - Fix indentation of new testcase
 - 8277072: ObjectStreamClass caches keep ClassLoaders alive

-

Changes: https://git.openjdk.java.net/jdk/pull/6637/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6637=01
  Stats: 431 lines in 4 files changed: 115 ins; 273 del; 43 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6637.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6637/head:pull/6637

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


Re: RFR: 8278065: Refactor subclassAudits to use ClassValue

2021-12-10 Thread openjdk-notifier[bot]
On Wed, 1 Dec 2021 14:45:23 GMT, Roman Kennke  wrote:

> As a follow-up to #6375, this change refactors 
> java.io.ObjectInputStream.Caches#subclassAudits and 
> java.io.ObjectOutputStream.Caches#subclassAudits to use ClassValue instead of 
> SoftReference, similar to what we did in #6375 for 
> java.io.ObjectStreamClass.Caches#localDescs. Then we can now also remove the 
> common machinery java.io.ObjectStreamClass#processQueue and 
> java.io.ObjectStreamClass.WeakClassKey.
> 
> Testing:
>  - [x] tier1
>  - [x] tier2
>  - [ ] tier3

The dependent pull request has now been integrated, and the target branch of 
this pull request has been updated. This means that changes from the dependent 
pull request can start to show up as belonging to this pull request, which may 
be confusing for reviewers. To remedy this situation, simply merge the latest 
changes from the new target branch into this pull request by running commands 
similar to these in the local repository for your personal fork:


git checkout JDK-8277072
git fetch https://git.openjdk.java.net/jdk master
git merge FETCH_HEAD
# if there are conflicts, follow the instructions given by git merge
git commit -m "Merge master"
git push

-

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


Integrated: 8277072: ObjectStreamClass caches keep ClassLoaders alive

2021-12-10 Thread Roman Kennke
On Fri, 12 Nov 2021 21:43:42 GMT, Roman Kennke  wrote:

> The caches in ObjectStreamClass basically map WeakReference to 
> SoftReference, where the ObjectStreamClass also references 
> the same Class. That means that the cache entry, and thus the class and its 
> class-loader, will not get reclaimed, unless the GC determines that memory 
> pressure is very high.
> 
> However, this seems bogus, because that unnecessarily keeps ClassLoaders and 
> all its classes alive much longer than necessary: as soon as a ClassLoader 
> (and all its classes) become unreachable, there is no point in retaining the 
> stuff in OSC's caches.
> 
> The proposed change is to use WeakReference instead of SoftReference for the 
> values in caches.
> 
> Testing:
>  - [x] tier1
>  - [x] tier2
>  - [x] tier3
>  - [ ] tier4

This pull request has now been integrated.

Changeset: 8eb453ba
Author:Roman Kennke 
URL:   
https://git.openjdk.java.net/jdk/commit/8eb453baebe377697286f7eb32280ca9f1fd7775
Stats: 496 lines in 4 files changed: 284 ins; 186 del; 26 mod

8277072: ObjectStreamClass caches keep ClassLoaders alive

Reviewed-by: rriggs, plevart

-

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


RFR: 8277481: Obsolete seldom used CDS flags

2021-12-10 Thread Harold Seigel
Please review this change to obsolete deprecated CDS options UseSharedSpaces, 
RequireSharedSpaces, DynamicDumpSharedSpaces, and DumpSharedSpaces.  The change 
was tested by running Mach5 tiers 1-2 on Linux, Mac OS, and Windows and Mach5 
tiers 3-5 on Linux x64 and Windows x64.

The use of UseSharedSpaces in ps_core_common.c was tested on Mac OS x64 by 
temporarily removing serviceability/sa/ClhsdbPmap.java#core from the problem 
list.

Thanks! Harold

-

Commit messages:
 - fix typo
 - 8277481: Obsolete seldom used CDS flags

Changes: https://git.openjdk.java.net/jdk/pull/6800/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6800=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8277481
  Stats: 151 lines in 13 files changed: 22 ins; 94 del; 35 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6800.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6800/head:pull/6800

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


Re: RFR: 8277072: ObjectStreamClass caches keep ClassLoaders alive [v8]

2021-12-10 Thread Roger Riggs
On Tue, 7 Dec 2021 10:42:50 GMT, Roman Kennke  wrote:

>> The caches in ObjectStreamClass basically map WeakReference to 
>> SoftReference, where the ObjectStreamClass also 
>> references the same Class. That means that the cache entry, and thus the 
>> class and its class-loader, will not get reclaimed, unless the GC determines 
>> that memory pressure is very high.
>> 
>> However, this seems bogus, because that unnecessarily keeps ClassLoaders and 
>> all its classes alive much longer than necessary: as soon as a ClassLoader 
>> (and all its classes) become unreachable, there is no point in retaining the 
>> stuff in OSC's caches.
>> 
>> The proposed change is to use WeakReference instead of SoftReference for the 
>> values in caches.
>> 
>> Testing:
>>  - [x] tier1
>>  - [x] tier2
>>  - [x] tier3
>>  - [ ] tier4
>
> Roman Kennke has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add improved test by @plevart

This fix hasn't had any bake-time and might have some effects that aren't 
immediately noticeable.
I'd leave it in 19 for the time being. It could be back ported at a later point 
in time.

-

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


Re: RFR: 8277072: ObjectStreamClass caches keep ClassLoaders alive [v8]

2021-12-10 Thread Roman Kennke
On Fri, 10 Dec 2021 14:44:07 GMT, Kevin Rushforth  wrote:

> This is a P4 bug. If the priority is correct, it does not meet the criteria 
> to get it into JDK 18 during RDP1, as indicated in [JEP 
> 3](https://openjdk.java.net/jeps/3).
> 
> If this is objectively a P3 bug, and really does need to go into JDK 18, then 
> you will need to close this PR and open a new pull request in the jdk18 repo.

Hmm, good question. It is kind-of leaking: current implementation prevents 
unloading of classes that are referenced from the OSC caches, unless memory 
pressure is high enough to trigger soft-ref-cleaning. Does it qualify for P3 
("Major loss of function."), or even P2 ("Crashes, loss of data, severe memory 
leak.")? We have users hitting this problem under different circumstances, I'd 
say it qualifies for P3. Opinions? See:

https://bugzilla.redhat.com/show_bug.cgi?id=2016930

-

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


Re: RFR: 8277072: ObjectStreamClass caches keep ClassLoaders alive [v8]

2021-12-10 Thread Kevin Rushforth
On Tue, 7 Dec 2021 10:42:50 GMT, Roman Kennke  wrote:

>> The caches in ObjectStreamClass basically map WeakReference to 
>> SoftReference, where the ObjectStreamClass also 
>> references the same Class. That means that the cache entry, and thus the 
>> class and its class-loader, will not get reclaimed, unless the GC determines 
>> that memory pressure is very high.
>> 
>> However, this seems bogus, because that unnecessarily keeps ClassLoaders and 
>> all its classes alive much longer than necessary: as soon as a ClassLoader 
>> (and all its classes) become unreachable, there is no point in retaining the 
>> stuff in OSC's caches.
>> 
>> The proposed change is to use WeakReference instead of SoftReference for the 
>> values in caches.
>> 
>> Testing:
>>  - [x] tier1
>>  - [x] tier2
>>  - [x] tier3
>>  - [ ] tier4
>
> Roman Kennke has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add improved test by @plevart

This is a P4 bug. If the priority is correct, it does not meet the criteria to 
get it into JDK 18 during RDP1, as indicated in [JEP 
3](https://openjdk.java.net/jeps/3).

If this is objectively a P3 bug, and really does need to go into JDK 18, then 
you will need to close this PR and open a new pull request in the jdk18 repo.

-

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


Re: RFR: 8277072: ObjectStreamClass caches keep ClassLoaders alive [v8]

2021-12-10 Thread Roman Kennke
On Tue, 7 Dec 2021 10:42:50 GMT, Roman Kennke  wrote:

>> The caches in ObjectStreamClass basically map WeakReference to 
>> SoftReference, where the ObjectStreamClass also 
>> references the same Class. That means that the cache entry, and thus the 
>> class and its class-loader, will not get reclaimed, unless the GC determines 
>> that memory pressure is very high.
>> 
>> However, this seems bogus, because that unnecessarily keeps ClassLoaders and 
>> all its classes alive much longer than necessary: as soon as a ClassLoader 
>> (and all its classes) become unreachable, there is no point in retaining the 
>> stuff in OSC's caches.
>> 
>> The proposed change is to use WeakReference instead of SoftReference for the 
>> values in caches.
>> 
>> Testing:
>>  - [x] tier1
>>  - [x] tier2
>>  - [x] tier3
>>  - [ ] tier4
>
> Roman Kennke has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add improved test by @plevart

This should go to openjdk/jdk18 now, right? Can I simply push it there, or do I 
need to re-open a PR against jdk18?

-

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


Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v23]

2021-12-10 Thread Andrew Leonard
On Fri, 10 Dec 2021 11:51:53 GMT, Lance Andersen  wrote:

>> Andrew Leonard has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   8276766: Enable jar and jmod to produce deterministic timestamped content
>>   
>>   Signed-off-by: Andrew Leonard 
>
> test/jdk/tools/jar/SourceDateDataProvider.java line 54:
> 
>> 52:};
>> 53: 
>> 54: @DataProvider(name = "SourceDateData.valid")
> 
> This class file is really not needed as you can just add the DataProvider to 
> ReproducableJar example:
> 
>  ```
>   @DataProvider
> private Object[][] invalidSourceDates() {
> return new Object[][] {
>  {"1976-06-24T01:02:03+00:00"},
>   {"1980-01-01T00:00:01+00:00"},
>   {"2100-01-01T00:00:00+00:00"},
>   {"2138-02-18T00:00:00-11:00"},
>   {"2006-04-06T12:38:00"},
>   {"2012-08-24T16"}
> };
> }
> 
> 
> And the Test will just use
> 
> `@Test(dataProvider= "invalidStourceDates")`

Thanks Lance, that is simpler, i've changed it

-

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


Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v24]

2021-12-10 Thread Andrew Leonard
> Add a new --source-date  (epoch seconds) option to jar and jmod to 
> allow specification of time to use for created/updated jar/jmod entries. This 
> then allows the ability to make the content deterministic.
> 
> Signed-off-by: Andrew Leonard 

Andrew Leonard has updated the pull request incrementally with one additional 
commit since the last revision:

  8276766: Enable jar and jmod to produce deterministic timestamped content
  
  Signed-off-by: Andrew Leonard 

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6481/files
  - new: https://git.openjdk.java.net/jdk/pull/6481/files/82e740e4..0a69d014

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=6481=23
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6481=22-23

  Stats: 99 lines in 2 files changed: 32 ins; 64 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6481.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6481/head:pull/6481

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


Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v23]

2021-12-10 Thread Lance Andersen
On Fri, 10 Dec 2021 10:54:06 GMT, Andrew Leonard  wrote:

>> Add a new --source-date  (epoch seconds) option to jar and jmod 
>> to allow specification of time to use for created/updated jar/jmod entries. 
>> This then allows the ability to make the content deterministic.
>> 
>> Signed-off-by: Andrew Leonard 
>
> Andrew Leonard has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8276766: Enable jar and jmod to produce deterministic timestamped content
>   
>   Signed-off-by: Andrew Leonard 

Hi Andrew,

Thank you for the changes.  This looks much much better.  Please see additional 
simplification comment below.

I kicked off a Mach5 run in the meantime

test/jdk/tools/jar/SourceDateDataProvider.java line 54:

> 52:};
> 53: 
> 54: @DataProvider(name = "SourceDateData.valid")

This class file is really not needed as you can just add the DataProvider to 
ReproducableJar example:

 ```
  @DataProvider
private Object[][] invalidSourceDates() {
return new Object[][] {
 {"1976-06-24T01:02:03+00:00"},
  {"1980-01-01T00:00:01+00:00"},
  {"2100-01-01T00:00:00+00:00"},
  {"2138-02-18T00:00:00-11:00"},
  {"2006-04-06T12:38:00"},
  {"2012-08-24T16"}
};
}


And the Test will just use

`@Test(dataProvider= "invalidStourceDates")`

-

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


Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v20]

2021-12-10 Thread Andrew Leonard
On Tue, 7 Dec 2021 19:25:05 GMT, Lance Andersen  wrote:

>> Andrew Leonard has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   8276766: Enable jar and jmod to produce deterministic timestamped content
>>   
>>   Signed-off-by: Andrew Leonard 
>
> Looks like the CSR has been approved.  I have a mach5 run that should 
> hopefully finish sooner rather than later and if that remains happy, I will 
> approve the PR

@LanceAndersen thank you for the review comments, i've just done a new commit 
with them all done.. assuming all the tests run ok, i'll integrate

-

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


Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v23]

2021-12-10 Thread Andrew Leonard
> Add a new --source-date  (epoch seconds) option to jar and jmod to 
> allow specification of time to use for created/updated jar/jmod entries. This 
> then allows the ability to make the content deterministic.
> 
> Signed-off-by: Andrew Leonard 

Andrew Leonard has updated the pull request incrementally with one additional 
commit since the last revision:

  8276766: Enable jar and jmod to produce deterministic timestamped content
  
  Signed-off-by: Andrew Leonard 

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6481/files
  - new: https://git.openjdk.java.net/jdk/pull/6481/files/0add0d3b..82e740e4

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=6481=22
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6481=21-22

  Stats: 6 lines in 1 file changed: 2 ins; 1 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6481.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6481/head:pull/6481

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


Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v21]

2021-12-10 Thread Andrew Leonard
On Thu, 9 Dec 2021 18:15:42 GMT, Lance Andersen  wrote:

>> Andrew Leonard has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   8276766: Enable jar and jmod to produce deterministic timestamped content
>>   
>>   Signed-off-by: Andrew Leonard 
>
> test/jdk/tools/jar/ReproducibleJar.java line 114:
> 
>> 112: }
>> 113: 
>> 114: @Test
> 
> Please add a comment introducing the intent of the test.  As mentioned above, 
> please consider using a DataProvider vs. an array with the test.

done

> test/jdk/tools/jar/ReproducibleJar.java line 286:
> 
>> 284: 
>> 285: static void extractJar(File jarFile) throws Throwable {
>> 286: String javahome = System.getProperty("java.home");
> 
> Please add a basic comment of the intent of the method.
> 
> Any reason you chose not to use JAR_TOOL here?

done

-

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


Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v22]

2021-12-10 Thread Andrew Leonard
> Add a new --source-date  (epoch seconds) option to jar and jmod to 
> allow specification of time to use for created/updated jar/jmod entries. This 
> then allows the ability to make the content deterministic.
> 
> Signed-off-by: Andrew Leonard 

Andrew Leonard has updated the pull request incrementally with one additional 
commit since the last revision:

  8276766: Enable jar and jmod to produce deterministic timestamped content
  
  Signed-off-by: Andrew Leonard 

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6481/files
  - new: https://git.openjdk.java.net/jdk/pull/6481/files/436762f2..0add0d3b

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=6481=21
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6481=20-21

  Stats: 220 lines in 2 files changed: 85 ins; 59 del; 76 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6481.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6481/head:pull/6481

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


Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v21]

2021-12-10 Thread Andrew Leonard
On Thu, 9 Dec 2021 18:31:57 GMT, Lance Andersen  wrote:

>> Andrew Leonard has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   8276766: Enable jar and jmod to produce deterministic timestamped content
>>   
>>   Signed-off-by: Andrew Leonard 
>
> test/jdk/tools/jar/ReproducibleJar.java line 74:
> 
>> 72: private static final File jarFileSourceDate1 = new 
>> File("JarEntryTimeSourceDate1.jar");
>> 73: private static final File jarFileSourceDate2 = new 
>> File("JarEntryTimeSourceDate2.jar");
>> 74: 
> 
> Please consider using caps for your static final variable names

done

-

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


Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v21]

2021-12-10 Thread Andrew Leonard
On Thu, 9 Dec 2021 18:40:01 GMT, Lance Andersen  wrote:

>> Andrew Leonard has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   8276766: Enable jar and jmod to produce deterministic timestamped content
>>   
>>   Signed-off-by: Andrew Leonard 
>
> test/jdk/tools/jar/ReproducibleJar.java line 30:
> 
>> 28:  *  reproducible
>> 29:  * @modules jdk.jartool
>> 30:  * @run testng ReproducibleJar
> 
> Probably use othervm given you are toggling the TZ for extra safety

done

> test/jdk/tools/jar/ReproducibleJar.java line 49:
> 
>> 47: import java.time.LocalDateTime;
>> 48: import java.time.ZonedDateTime;
>> 49: import java.time.ZoneId;
> 
> Duplicate import?  Perhaps get rid to the import of line 43

done

> test/jdk/tools/jar/ReproducibleJar.java line 102:
> 
>> 100: jarFileSourceDate1.delete();
>> 101: jarFileSourceDate2.delete();
>> 102: TimeZone.setDefault(TZ);
> 
> I believe you could just call runAfter() from within this method

done

> test/jdk/tools/jar/ReproducibleJar.java line 120:
> 
>> 118: // Test --date source date
>> 119: for (String sourceDate : sourceDates) {
>> 120: createOuterInnerDirs(dirOuter, dirInner);
> 
> If you use a DataProvider,  you could call this method from your 
> runBeforeMethod

done

> test/jdk/tools/jar/ReproducibleJar.java line 147:
> 
>> 145: cleanup(dirInner);
>> 146: cleanup(dirOuter);
>> 147: jarFileSourceDate1.delete();
> 
> Is the above needed given your  `@AfterMethod`

done

> test/jdk/tools/jar/ReproducibleJar.java line 152:
> 
>> 150: 
>> 151: @Test
>> 152: public void testInvalidSourceDate() throws Throwable {
> 
> Please add a basic comment of the intent of the method and consider using a 
> DataProvider

done

> test/jdk/tools/jar/ReproducibleJar.java line 165:
> 
>> 163: 
>> 164: @Test
>> 165: public void testJarsReproducible() throws Throwable {
> 
> Please add a basic comment of the intent of the method and consider using a 
> DataProvider

done

> test/jdk/tools/jar/ReproducibleJar.java line 199:
> 
>> 197: jarFileSourceDate1.delete();
>> 198: jarFileSourceDate2.delete();
>> 199: }
> 
> I believe your` @AfterMethod` will address this so the above is not needed

done

> test/jdk/tools/jar/ReproducibleJar.java line 202:
> 
>> 200: }
>> 201: 
>> 202: static void createOuterInnerDirs(File dirOuter, File dirInner) 
>> throws Throwable {
> 
> I believe you are always passing in the same parameter values, so perhaps no 
> need for the parameters?

done

> test/jdk/tools/jar/ReproducibleJar.java line 208:
> 
>> 206:  * foo.txt
>> 207:  */
>> 208: Assert.assertTrue(dirOuter.mkdir());
> 
> Please move the  comment above the method

done

> test/jdk/tools/jar/ReproducibleJar.java line 249:
> 
>> 247: }
>> 248: 
>> 249: private static boolean isTimeSettingChanged() {
> 
> Please add a basic comment describing the intent of the method

done

> test/jdk/tools/jar/ReproducibleJar.java line 260:
> 
>> 258: }
>> 259: 
>> 260: private static boolean isInTransition() {
> 
> Please add a basic comment of the intent of the method

done

> test/jdk/tools/jar/ReproducibleJar.java line 278:
> 
>> 276: File[] x = dir.listFiles();
>> 277: if (x != null) {
>> 278: for (int i = 0; i < x.length; i++) {
> 
> Could use enhanced for loop here if you desire

done

-

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


Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v21]

2021-12-10 Thread Andrew Leonard
On Thu, 9 Dec 2021 18:14:39 GMT, Lance Andersen  wrote:

>> Andrew Leonard has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   8276766: Enable jar and jmod to produce deterministic timestamped content
>>   
>>   Signed-off-by: Andrew Leonard 
>
> test/jdk/tools/jar/ReproducibleJar.java line 95:
> 
>> 93:  "2006-04-06T12:38:00",
>> 94:  "2012-08-24T16"};
>> 95: 
> 
> Another good use of a DataProvider

done

> test/jdk/tools/jar/ReproducibleJar.java line 193:
> 
>> 191: 
>> 192: // Check jars are identical
>> 193: checkSameContent(jarFileSourceDate1, jarFileSourceDate2);
> 
> You could if you choose use:
> 
>  Assert.assertEquals(Files.readAllBytes(jarFileSourceDate1.toPath(), 
> Files.readAllBytes(jarFileSourceDate2.toPath()));

done

> test/jdk/tools/jar/ReproducibleJar.java line 219:
> 
>> 217: }
>> 218: 
>> 219: static void checkFileTime(long now, long original) throws Throwable 
>> {
> 
> Please add a basic comment of the intent of the method

done

> test/jdk/tools/jar/ReproducibleJar.java line 241:
> 
>> 239: }
>> 240: 
>> 241: static void checkSameContent(File f1, File f2) throws Throwable {
> 
> See comment above for consideration  regarding the use of Assert.assertEquals

done

-

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


Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v21]

2021-12-10 Thread Andrew Leonard
On Thu, 9 Dec 2021 22:11:57 GMT, Andrew Leonard  wrote:

>> test/jdk/tools/jar/ReproducibleJar.java line 87:
>> 
>>> 85: "2098-02-18T00:00:00-08:00",
>>> 86: "2099-12-31T23:59:59+00:00"};
>>> 87: 
>> 
>> I would suggest converting to a DataProvider for the test.
>
> thanks, that's neat, not used a @DataProvider before

done

-

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


Re: RFR: 8269556: sun/tools/jhsdb/JShellHeapDumpTest.java fails with RuntimeException 'JShellToolProvider' missing from stdout/stderr

2021-12-10 Thread Kevin Walls
On Fri, 10 Dec 2021 07:00:10 GMT, Chris Plummer  wrote:

> The test searches for "JShellToolProvider" in the main thread's stack trace, 
> which is pulled from an SA heap dump. Typically the main thread is blocked in 
> Object.wait(), so SA can determine its stack trace. However, the wait has a 
> 100ms timeout, so the thread does periodically wake up and does a few things 
> before waiting again. If SA tries to get the stack trace while the thread is 
> active, it may not be able to, and this causes the test to fail. I determined 
> this only happens about 1 in 5000-1 runs. So as a fix I'm allowing the 
> test to do one retry. That should make it extremely unlikely that we ever see 
> this failure again. I also bumped up the amount of time the test waits before 
> doing the heap dump from 2 seconds to 4 just to make absolutely sure the main 
> thread is fully started before doing the heap dump.

Looks good.

-

Marked as reviewed by kevinw (Committer).

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


Re: RFR: 8271079: JavaFileObject#toUri and multi-release jars [v3]

2021-12-10 Thread Alan Bateman
On Fri, 10 Dec 2021 08:16:46 GMT, Christian Stein  wrote:

>> Prior to this PR, `toUri()` of class `ZipPath` in module `jdk.zipfs` and 
>> class `PathFileObject` in module `jdk.compiler` were always composed by base 
>> path names. Even for versioned entries of a multi-release JAR file.
>> 
>> Now, a `URI` for an entry is composed of its real path names using an 
>> existing lookup function in the associated zip file system object.
>> 
>> This PR also removes a superseded work around for 
>> [JDK-8134451](https://bugs.openjdk.java.net/browse/JDK-8134451).
>> 
>> Fixes https://bugs.openjdk.java.net/browse/JDK-8271079
>
> Christian Stein has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Turn `getRealPath()` into a no-arg helper method

The changes to zipfs in the latest revision look good

-

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


Re: RFR: 8271079: JavaFileObject#toUri and multi-release jars [v2]

2021-12-10 Thread Christian Stein
On Thu, 9 Dec 2021 18:37:11 GMT, Alan Bateman  wrote:

>> Christian Stein has updated the pull request incrementally with two 
>> additional commits since the last revision:
>> 
>>  - Clean up reproducer test case
>>  - Keep helper in ZipFileSystem simple
>
> src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipPath.java line 211:
> 
>> 209: 
>> 210: private String getRealPath(byte[] resolvedPath) {
>> 211: byte[] path = zfs.lookupPath(resolvedPath);
> 
> Can getRealPath be changed to be no-arg method that calls getResolvedPath and 
> then does the lookup. I think that would be a clearer than assuming the 
> argument is a resolved path.

Done via 
https://github.com/openjdk/jdk/pull/6768/commits/12e106c6e69449f1fbde9e5b6a5ff3305c5de547

-

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


Re: RFR: 8271079: JavaFileObject#toUri and multi-release jars [v3]

2021-12-10 Thread Christian Stein
> Prior to this PR, `toUri()` of class `ZipPath` in module `jdk.zipfs` and 
> class `PathFileObject` in module `jdk.compiler` were always composed by base 
> path names. Even for versioned entries of a multi-release JAR file.
> 
> Now, a `URI` for an entry is composed of its real path names using an 
> existing lookup function in the associated zip file system object.
> 
> This PR also removes a superseded work around for 
> [JDK-8134451](https://bugs.openjdk.java.net/browse/JDK-8134451).
> 
> Fixes https://bugs.openjdk.java.net/browse/JDK-8271079

Christian Stein has updated the pull request incrementally with one additional 
commit since the last revision:

  Turn `getRealPath()` into a no-arg helper method

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6768/files
  - new: https://git.openjdk.java.net/jdk/pull/6768/files/e62721a5..12e106c6

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=6768=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6768=01-02

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

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