Integrated: 8276657: XSLT compiler tries to define a class with empty name
On Tue, 30 Nov 2021 18:53:26 GMT, Joe Wang wrote: > The result of Util.baseName(systemId) can be empty, causing the compiler to > set an empty classname. Add a check to make sure it will not set the empty > classname. > > Alternatively, it may report an error, but that would be disruptive. As the > transform can proceed without the provided classname (by using the default), > adding a check is better than reporting an error. > > I've verified the patch with the proposed fix for JDK-8276241. Harold has > also confirmed it fixes the tests in his builds. This pull request has now been integrated. Changeset: a093cddd Author:Joe Wang URL: https://git.openjdk.java.net/jdk/commit/a093cdddaf5ab88eb84a147e523db5c3e1be54be Stats: 6 lines in 1 file changed: 3 ins; 0 del; 3 mod 8276657: XSLT compiler tries to define a class with empty name Reviewed-by: naoto - PR: https://git.openjdk.java.net/jdk/pull/6620
Re: RFR: JDK-8277375: jdeps errors on a class path with a file path with no permission
> On Nov 29, 2021, at 1:14 PM, Mandy Chung wrote: > > On Wed, 24 Nov 2021 12:32:32 GMT, Michael Hall wrote: > >> Would there be any need to scan class path at all? That would mean a module >> would have a class path dependency wouldn't it? > > One reason of scanning the class path is to detect any split packages and > emit warnings. > >> Yes, I shouldn't of included -cp at all but shouldn't keeps ignore it? > > In general, a module should not depend on any class on the classpath. > However, it's possible that there is any missing dependency i.e. can't be > found from its required modules. One could use `-cp` to add additional > libraries for a quick analysis. > > I got your point though. Scanning class path is not strictly necessary to > analyze dependencies and it can be done lazily. Finding all packages in the > unnamed module is just the current implementation. I'll consider and look > into making it lazy. > > - > > PR: https://git.openjdk.java.net/jdk/pull/6531 Sorry, Didn’t see this. Thanks for the reply.
Integrated: 8276141: XPathFactory set/getProperty method
On Tue, 2 Nov 2021 17:31:40 GMT, Joe Wang wrote: > Add setProperty/getProperty methods to the XPath API so that properties can > be supported in the future. This pull request has now been integrated. Changeset: b226ab99 Author:Joe Wang URL: https://git.openjdk.java.net/jdk/commit/b226ab99c872e791d3ed9fca015cf92847904c34 Stats: 258 lines in 5 files changed: 247 ins; 3 del; 8 mod 8276141: XPathFactory set/getProperty method Reviewed-by: rriggs, naoto, lancea, iris, alanb - PR: https://git.openjdk.java.net/jdk/pull/6215
Re: RFR: 8277422: tools/jar/JarEntryTime.java fails with modified time mismatch
On Wed, 1 Dec 2021 18:33:44 GMT, Lance Andersen wrote: > Hi all, > > Please review this patch to address a failure at DST->STD offset transition. > The fix mirrors what was done for JDK-8190748 > > > Best, > Lance Marked as reviewed by joehw (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6648
Re: RFR: JDK-8276674 : Malformed Javadoc inline tags in JDK source in java/util/stream [v2]
> JDK-8276674 : Malformed Javadoc inline tags in JDK source in java/util/stream Tim Prinzing has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains two commits: - Merge branch 'master' into JDK-8276674 - Fixed @code and @link in some javadoc for JDK-8276674 - Changes: https://git.openjdk.java.net/jdk/pull/6443/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6443=01 Stats: 13 lines in 8 files changed: 0 ins; 0 del; 13 mod Patch: https://git.openjdk.java.net/jdk/pull/6443.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6443/head:pull/6443 PR: https://git.openjdk.java.net/jdk/pull/6443
Re: RFR: 8277992: Add fast jdk_svc subtests to jdk:tier3
On Thu, 2 Dec 2021 02:00:11 GMT, Mikhailo Seledtsov wrote: >> Hi @shipilev , >> We need to have someone look at the impact of this on our CI. I don't think >> we run the tier4 group as defined in our tier 4 so we may not see those >> execution time savings that offset the extra cost to tier3. > > Thanks @dholmes-ora for making sure to examine the impact on the CI. I took a > look, and this change should not have considerable adverse impact on Oracle > CI. > > Aleksey, the change looks good to me. Thanks @mseledts for examining - much appreciated. Thanks for your patience @shipilev . - PR: https://git.openjdk.java.net/jdk/pull/6619
Re: RFR: 8277992: Add fast jdk_svc subtests to jdk:tier3
On Wed, 1 Dec 2021 02:29:14 GMT, David Holmes wrote: >> OpenJDK tiered tests definitions have the catch-all `tier4` that runs all >> tests not defined in the lower tiers. `hotspot:tier4` has lots of them, >> mostly long-running vmTestbase tests, which take many hours even on a very >> parallel machines. >> >> This, unfortunately, has a chilling effect on `jdk:tier4`, which is seldom >> run by contributors, because `hotspot:tier4` is in the way. But, there are >> plenty of fast and stable tests in `jdk:tier4` that can be run in >> `jdk:tier3`. `jdk_svc` is the example of such tests: management features >> (including but not limited to JFR) are important to keep from regressions, >> and significant subset of them runs pretty fast. >> >> So, it makes sense to move some `jdk_svc` tests to `jdk:tier3` to expose it >> to more contributors. I think the only group we don't want to run is >> `svc_tools`, which includes lots of non-parallel tests that are rather slow. >> >> Sample run before: >> >> >> == >> Test summary >> == >>TEST TOTAL PASS FAIL ERROR >> >>jtreg:test/jdk:tier3174 174 0 0 >> >> == >> TEST SUCCESS >> >> Finished building target 'run-test' in configuration >> 'linux-x86_64-server-fastdebug' >> >> real 2m38.242s >> user 80m7.216s >> sys 2m13.846s >> >> >> == >> Test summary >> == >>TEST TOTAL PASS FAIL ERROR >> jtreg:test/jdk:tier4 2904 2901 3 0 << >> == >> TEST FAILURE >> >> real 18m13.933s >> user 546m50.556s >> sys 25m7.086s >> >> >> Sample run after: >> >> >> == >> Test summary >> == >>TEST TOTAL PASS FAIL ERROR >> >>jtreg:test/jdk:tier3 1296 1296 0 0 >> >> == >> TEST SUCCESS >> >> Finished building target 'run-test' in configuration >> 'linux-x86_64-server-fastdebug' >> >> real 7m49.017s >> user 287m30.943s >> sys 13m20.060s >> >> == >> Test summary >> == >>TEST TOTAL PASS FAIL ERROR >> jtreg:test/jdk:tier4 1783 1780 3 0 << >> == >> TEST FAILURE >> >> >> real 12m19.973s >> user 351m48.561s >> sys 14m51.566s > > Hi @shipilev , > We need to have someone look at the impact of this on our CI. I don't think > we run the tier4 group as defined in our tier 4 so we may not see those > execution time savings that offset the extra cost to tier3. Thanks @dholmes-ora for making sure to examine the impact on the CI. I took a look, and this change should not have considerable adverse impact on Oracle CI. Aleksey, the change looks good to me. - PR: https://git.openjdk.java.net/jdk/pull/6619
Re: RFR: 8277992: Add fast jdk_svc subtests to jdk:tier3
On Tue, 30 Nov 2021 18:48:15 GMT, Aleksey Shipilev wrote: > OpenJDK tiered tests definitions have the catch-all `tier4` that runs all > tests not defined in the lower tiers. `hotspot:tier4` has lots of them, > mostly long-running vmTestbase tests, which take many hours even on a very > parallel machines. > > This, unfortunately, has a chilling effect on `jdk:tier4`, which is seldom > run by contributors, because `hotspot:tier4` is in the way. But, there are > plenty of fast and stable tests in `jdk:tier4` that can be run in > `jdk:tier3`. `jdk_svc` is the example of such tests: management features > (including but not limited to JFR) are important to keep from regressions, > and significant subset of them runs pretty fast. > > So, it makes sense to move some `jdk_svc` tests to `jdk:tier3` to expose it > to more contributors. I think the only group we don't want to run is > `svc_tools`, which includes lots of non-parallel tests that are rather slow. > > Sample run before: > > > == > Test summary > == >TEST TOTAL PASS FAIL ERROR > >jtreg:test/jdk:tier3174 174 0 0 > > == > TEST SUCCESS > > Finished building target 'run-test' in configuration > 'linux-x86_64-server-fastdebug' > > real 2m38.242s > user 80m7.216s > sys 2m13.846s > > > == > Test summary > == >TEST TOTAL PASS FAIL ERROR > >>> jtreg:test/jdk:tier4 2904 2901 3 0 << > == > TEST FAILURE > > real 18m13.933s > user 546m50.556s > sys 25m7.086s > > > Sample run after: > > > == > Test summary > == >TEST TOTAL PASS FAIL ERROR > >jtreg:test/jdk:tier3 1296 1296 0 0 > > == > TEST SUCCESS > > Finished building target 'run-test' in configuration > 'linux-x86_64-server-fastdebug' > > real 7m49.017s > user 287m30.943s > sys 13m20.060s > > == > Test summary > == >TEST TOTAL PASS FAIL ERROR > >>> jtreg:test/jdk:tier4 1783 1780 3 0 << > == > TEST FAILURE > > > real 12m19.973s > user 351m48.561s > sys 14m51.566s Marked as reviewed by mseledtsov (Committer). - PR: https://git.openjdk.java.net/jdk/pull/6619
Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v14]
On Wed, 1 Dec 2021 21:50:46 GMT, Andrew Leonard wrote: > I can adjust the --date value validation to be > 1980-01-01T00:00:00Z ? I know, nit picky, right? I'll take no offense if you ignore it. The thing is, everyone else implementing this feature gave that specific instant a wide margin. (See my previous comment about Debian and Gradle.) Debian adds 12 hours 1 minute just to avoid it (`1980-01-01T12:01:00Z`), while Gradle even skips to the next month (`1980-02-01T00:00:00`). Because developers looking into this discover the magic earliest date, some may think it's a great choice for a default deterministic value and use it! Then this current implementation breaks with no warning. If you add one second, the `ZipEntry` method rounds down, and you get the exact same "DOS date and time" in the output (1980 Jan 1 00:00:00), but now it works. Kind of confusing. So my latest recommendation is to define the earliest permitted instant at `1980-01-01T00:00:02Z` to avoid any possibility of setting the magic local date and time at all. The Gradle folks might explain it better in [their comment here][1]. [1]: https://github.com/gradle/gradle/blob/master/subprojects/core/src/main/java/org/gradle/api/internal/file/archive/ZipCopyAction.java#L42-L56 - PR: https://git.openjdk.java.net/jdk/pull/6481
Re: RFR: 8277861: Terminally deprecate Thread.stop
On Tue, 30 Nov 2021 14:52:37 GMT, Alan Bateman wrote: > Thread.stop is inherently unsafe and has been deprecated since Java 1.2 > (1998). It's time to terminally deprecate this method so it can be degraded > and removed in the future. > > This PR does not propose any changes to the JVM TI StopThread function (or > the corresponding JDWP command or JDI method). Endorsed. - PR: https://git.openjdk.java.net/jdk/pull/6616
Re: RFR: 8277647: [REDO] JDK-8277507 Add jlink.debug system property while launching jpackage tests to help diagonize recent intermittent failures
On Wed, 1 Dec 2021 12:21:00 GMT, Athijegannathan Sundararajan wrote: >> Can I please get a review of this change which adds `jlink.debug=true` >> system property while launching `jpackage` tests? >> >> The previous fix for this in https://github.com/openjdk/jdk/pull/6491 didn't >> take into account the part where the `jpackage` tool gets launched as a >> `ToolProvider` from some of these tests. This resulted in a large number of >> tests failing (across different OS) in `tier2` with errors like: >> >> >> Error: Invalid Option: [-J-Djlink.debug=true] >> >> >> In this current PR, the changed code now takes into account the possibility >> of `jpackage` being launched as a `ToolProvider` and in such cases doesn't >> add this option. To achieve this, the adding of this argument is delayed >> till when the actual execution is about to happen and thus it's now done in >> the `adjustArgumentsBeforeExecution()` method of the jpackage test framework. >> >> With this change, I have now run the `jdk:tier2` locally on a macos instance >> and the tests have all passed: >> >> >> Test results: passed: 3,821; failed: 3 >> Report written to >> jdk/build/macosx-x86_64-server-release/test-results/jtreg_test_jdk_tier2/html/report.html >> Results written to >> jdk/build/macosx-x86_64-server-release/test-support/jtreg_test_jdk_tier2 >> Error: Some tests failed or other problems occurred. >> Finished running test 'jtreg:test/jdk:tier2' >> Test report is stored in >> build/macosx-x86_64-server-release/test-results/jtreg_test_jdk_tier2 >> >> == >> Test summary >> == >>TEST TOTAL PASS FAIL ERROR >> jtreg:test/jdk:tier2 3824 3821 3 0 << >> == >> >> The 3 failing tests are unrelated to this change and belong to the >> `java/nio/channels/DatagramChannel/` test package. >> Furthermore, I've looked into the generated logs of the following tests to >> verify that the `-J-Djlink.debug=true` does get passed in relevant tests and >> doesn't in those that failed previously in `tier2`: >> >> test/jdk/tools/jpackage/share/MultiLauncherTwoPhaseTest.java >> test/jdk/tools/jpackage/macosx/NameWithSpaceTest.java >> test/jdk/tools/jpackage/share/ArgumentsTest.java >> >> A sample from one of the logs where this system property is expected to be >> passed along: >> >>> TRACE: exec: Execute >>> [jdk/build/macosx-x86_64-server-release/images/jdk/bin/jpackage --input >>> ./test/input --dest ./test/output --name "Name With Space" --type pkg >>> --main-jar hello.jar --main-class Hello -J-Djlink.debug=true >>> --verbose](15); inherit I/O... >> >> >> I would still like to request someone with access to CI or other OSes (like >> Windows and Linux) to help test `tier2` against this PR. > > LGTM Thank you for the review @sundararajana - PR: https://git.openjdk.java.net/jdk/pull/6534
Integrated: 8277647: [REDO] JDK-8277507 Add jlink.debug system property while launching jpackage tests to help diagonize recent intermittent failures
On Wed, 24 Nov 2021 08:54:08 GMT, Jaikiran Pai wrote: > Can I please get a review of this change which adds `jlink.debug=true` system > property while launching `jpackage` tests? > > The previous fix for this in https://github.com/openjdk/jdk/pull/6491 didn't > take into account the part where the `jpackage` tool gets launched as a > `ToolProvider` from some of these tests. This resulted in a large number of > tests failing (across different OS) in `tier2` with errors like: > > > Error: Invalid Option: [-J-Djlink.debug=true] > > > In this current PR, the changed code now takes into account the possibility > of `jpackage` being launched as a `ToolProvider` and in such cases doesn't > add this option. To achieve this, the adding of this argument is delayed till > when the actual execution is about to happen and thus it's now done in the > `adjustArgumentsBeforeExecution()` method of the jpackage test framework. > > With this change, I have now run the `jdk:tier2` locally on a macos instance > and the tests have all passed: > > > Test results: passed: 3,821; failed: 3 > Report written to > jdk/build/macosx-x86_64-server-release/test-results/jtreg_test_jdk_tier2/html/report.html > Results written to > jdk/build/macosx-x86_64-server-release/test-support/jtreg_test_jdk_tier2 > Error: Some tests failed or other problems occurred. > Finished running test 'jtreg:test/jdk:tier2' > Test report is stored in > build/macosx-x86_64-server-release/test-results/jtreg_test_jdk_tier2 > > == > Test summary > == >TEST TOTAL PASS FAIL ERROR > >>> jtreg:test/jdk:tier2 3824 3821 3 0 << > == > > The 3 failing tests are unrelated to this change and belong to the > `java/nio/channels/DatagramChannel/` test package. > Furthermore, I've looked into the generated logs of the following tests to > verify that the `-J-Djlink.debug=true` does get passed in relevant tests and > doesn't in those that failed previously in `tier2`: > > test/jdk/tools/jpackage/share/MultiLauncherTwoPhaseTest.java > test/jdk/tools/jpackage/macosx/NameWithSpaceTest.java > test/jdk/tools/jpackage/share/ArgumentsTest.java > > A sample from one of the logs where this system property is expected to be > passed along: > >> TRACE: exec: Execute >> [jdk/build/macosx-x86_64-server-release/images/jdk/bin/jpackage --input >> ./test/input --dest ./test/output --name "Name With Space" --type pkg >> --main-jar hello.jar --main-class Hello -J-Djlink.debug=true --verbose](15); >> inherit I/O... > > > I would still like to request someone with access to CI or other OSes (like > Windows and Linux) to help test `tier2` against this PR. This pull request has now been integrated. Changeset: 09522db5 Author:Jaikiran Pai URL: https://git.openjdk.java.net/jdk/commit/09522db5aa9503131381bbb4fe3f2eae829645ce Stats: 5 lines in 1 file changed: 5 ins; 0 del; 0 mod 8277647: [REDO] JDK-8277507 Add jlink.debug system property while launching jpackage tests to help diagonize recent intermittent failures Reviewed-by: sundar - PR: https://git.openjdk.java.net/jdk/pull/6534
Re: RFR: JDK-8276837: [macos]: Error when signing the additional launcher
On Wed, 1 Dec 2021 21:35:01 GMT, Andy Herrick wrote: > - We need to unsign all executables and libraries in the app-image before > signing. (not just those in the runtime). > - Clean up excessive output by executing the individual file sign and > unsigning commands in quiet mode. > - Add conditions in SigningAppImageTest to test signing of additional > launchers. Marked as reviewed by almatvee (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6654
Re: RFR: JDK-8276447 Deprecate finalization-related methods for removal [v3]
On Wed, 1 Dec 2021 19:23:59 GMT, Brent Christian wrote:
>> Here are the code changes for the "Deprecate finalizers in the standard Java
>> API" portion of JEP 421 ("Deprecate Finalization for Removal") for code
>> review.
>>
>> This change makes the indicated deprecations, and updates the API spec for
>> JEP 421. It also updates the relevant @SuppressWarning annotations.
>>
>> The CSR has been approved.
>> An automated test build+test run passes cleanly (FWIW :D ).
>
> Brent Christian has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains 33 commits:
>
> - Merge branch 'master' into 8276447
> - Merge branch 'master' into 8276447
> - merge
> - @SuppressWarnings(removal) in Windows CKey.java
> - Add jls tag to runFinalization methods
> - Update wording of Object.finalize, new paragraph is now bold
> - update Object.finalize javadoc
> - update Object.finalize JavaDoc and @deprecated tag
> - Update Object.finalize deprecation message
> - Indicate that runFinalizers does nothing if finalization is disabled or
> removed
> - ... and 23 more:
> https://git.openjdk.java.net/jdk/compare/0dfb3a70...8cde0674
Marked as reviewed by smarks (Reviewer).
src/jdk.jconsole/share/classes/sun/tools/jconsole/SummaryTab.java line 116:
> 114: StringBuilder buf;
> 115:
> 116: @SuppressWarnings("deprecation")
Item for future cleanup: refactor the call to
`getObjectPendingFinalizationCount()` at line 219 (!) into a local variable
declaration, and then move the warnings suppression to that declaration. This
reduces the scope of warnings suppression.
-
PR: https://git.openjdk.java.net/jdk/pull/6465
Re: RFR: JDK-8276837: [macos]: Error when signing the additional launcher
On Wed, 1 Dec 2021 21:35:01 GMT, Andy Herrick wrote: > - We need to unsign all executables and libraries in the app-image before > signing. (not just those in the runtime). > - Clean up excessive output by executing the individual file sign and > unsigning commands in quiet mode. > - Add conditions in SigningAppImageTest to test signing of additional > launchers. Marked as reviewed by asemenyuk (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6654
Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v14]
On Wed, 1 Dec 2021 18:02:05 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:
>
> Update src/jdk.jlink/share/classes/jdk/tools/jmod/JmodOutputStream.java
>
> Co-authored-by: Magnus Ihse Bursie
Mainly just two minor comments: (1) there's a better date and time parser, and
(2) the current lower bound is a couple seconds off. I'll reply to your
question about (2) separately.
The `DateTimeFormatter.ISO_ZONED_DATE_TIME` zoned date and time parser works
better than the `ISO_DATE_TIME` local date and time parser when creating a
`ZonedDateTime` instance. It catches the following error slightely earlier and
provides a better error message.
Here's the current error using the local date and time parser:
$ jmod create --date 2011-12-03T10:15:30 ...
Error: --date 2011-12-03T10:15:30 is not a valid ISO 8601 date and time:
Text '2011-12-03T10:15:30' could not be parsed:
Unable to obtain ZonedDateTime from TemporalAccessor:
{},ISO resolved to 2011-12-03T10:15:30 of type java.time.format.Parsed
Here's the same error using the zoned date and time parser:
$ jmod create --date 2011-12-03T10:15:30 ...
Error: --date 2011-12-03T10:15:30 is not a valid ISO 8601 date and time:
Text '2021-11-29T09:36:06' could not be parsed at index 19
src/jdk.jartool/share/classes/sun/tools/jar/GNUStyleOptions.java line 199:
> 197: void process(Main jartool, String opt, String arg)
> throws BadArgs {
> 198: try {
> 199: jartool.date = ZonedDateTime.parse(arg,
> DateTimeFormatter.ISO_DATE_TIME)
The `ISO_ZONED_DATE_TIME` parser works better here.
src/jdk.jartool/share/classes/sun/tools/jar/GNUStyleOptions.java line 201:
> 199: jartool.date = ZonedDateTime.parse(arg,
> DateTimeFormatter.ISO_DATE_TIME)
> 200:
> .withZoneSameInstant(ZoneOffset.UTC).toLocalDateTime();
> 201: if (jartool.date.getYear() < 1980 ||
> jartool.date.getYear() > 2099) {
This `if` test actually compares down to the nanosecond, if present, but the
wrong nanosecond. How about something like the following?
... ZonedDateTime TIME_MIN = ZonedDateTime.parse("1980-01-01T00:00:02Z");
... ZonedDateTime TIME_MAX = ZonedDateTime.parse("2099-12-31T23:59:59Z");
...
var zoned = ZonedDateTime.parse(date,
DateTimeFormatter.ISO_ZONED_DATE_TIME);
if (zoned.isBefore(TIME_MIN) || zoned.isAfter(TIME_MAX)) {
src/jdk.jartool/share/classes/sun/tools/jar/resources/jar.properties line 88:
> 86: date {0} is not a valid ISO 8601 date and time
> 87: error.date.out.of.range=\
> 88: date {0} is not within the valid year range 1980->2099
Maybe just define `date {0} is not within the valid range` and put the exact
interval in the *man* pages and documentation?
src/jdk.jlink/share/classes/jdk/tools/jmod/JmodTask.java line 1164:
> 1162: public LocalDateTime convert(String value) {
> 1163: try {
> 1164: LocalDateTime date = ZonedDateTime.parse(value,
> DateTimeFormatter.ISO_DATE_TIME)
The `ISO_ZONED_DATE_TIME` parser works better here.
src/jdk.jlink/share/classes/jdk/tools/jmod/JmodTask.java line 1166:
> 1164: LocalDateTime date = ZonedDateTime.parse(value,
> DateTimeFormatter.ISO_DATE_TIME)
> 1165:
> .withZoneSameInstant(ZoneOffset.UTC).toLocalDateTime();
> 1166: if (date.getYear() < 1980 || date.getYear() > 2099) {
See previous comment for this line in the `jar` tool.
src/jdk.jlink/share/classes/jdk/tools/jmod/resources/jmod.properties line 111:
> 109: err.no.moduleToHash=No hashes recorded: no module matching {0} found to
> record hashes
> 110: err.invalid.date=--date {0} is not a valid ISO 8601 date and time: {1}
> 111: err.date.out.of.range=--date {0} is out of the valid year range
> 1980->2099
As before, perhaps refer to the exact range in the documentation? It's really
an instant on the time line that is being compared and not the actual year
digits that were provided in the command-line option.
test/jdk/tools/jar/JarEntryTime.java line 180:
> 178: "2022-03-15T00:00:00+06:00",
> 179: "2038-11-26T06:06:06+00:00",
> 180: "2098-02-18T00:00:00-08:00"};
It would be great to test just inside the exact boundaries of the permitted
interval here.
test/jdk/tools/jar/JarEntryTime.java line 251:
> 249: // Negative Tests --date out of range source date
> 250: String[]
RFR: 8277422: tools/jar/JarEntryTime.java fails with modified time mismatch
Hi all, Please review this patch to address a failure at DST->STD offset transition. The fix mirrors what was done for JDK-8190748 Best, Lance - Commit messages: - Patch for JDK-8277422 Changes: https://git.openjdk.java.net/jdk/pull/6648/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6648=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8277422 Stats: 11 lines in 1 file changed: 9 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/6648.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6648/head:pull/6648 PR: https://git.openjdk.java.net/jdk/pull/6648
Re: RFR: 8278080: Add --with-cacerts-src='user cacerts folder' to enable determinsitic cacerts generation
On Wed, 1 Dec 2021 18:30:06 GMT, Andrew Leonard wrote: > Addition of a configure option --with-cacerts-src='user cacerts folder' to > allow developers to specify their own cacerts PEM folder for generation of > the cacerts store using the deterministic openjdk GenerateCacerts tool. > > Signed-off-by: Andrew Leonard I have a question related to the custom cacerts which can be added to the OpenJDK bundle. How do you pass the tests like test/jdk/sun/security/lib/cacerts/VerifyCACerts.java using that custom jdk bundle? Probably we can add an additional configuration to that test so it will check the custom cacerts passed to the build as well? - PR: https://git.openjdk.java.net/jdk/pull/6647
RFR: JDK-8276837: [macos]: Error when signing the additional launcher
- We need to unsign all executables and libraries in the app-image before signing. (not just those in the runtime). - Clean up excessive output by executing the individual file sign and unsigning commands in quiet mode. - Add conditions in SigningAppImageTest to test signing of additional launchers. - Commit messages: - JDK-8276837: [macos]: Error when signing the additional launcher - JDK-8276837: [macos]: Errorwhen signing the additional launcher Changes: https://git.openjdk.java.net/jdk/pull/6654/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6654=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8276837 Stats: 52 lines in 3 files changed: 21 ins; 5 del; 26 mod Patch: https://git.openjdk.java.net/jdk/pull/6654.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6654/head:pull/6654 PR: https://git.openjdk.java.net/jdk/pull/6654
Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v14]
On Wed, 1 Dec 2021 18:20:11 GMT, John Neffenger wrote: > I'm testing it now. So far, it's working great! > > I found one boundary problem. The magic instant of `1980-01-01T00:00:00Z` > triggers an extended header with the wrong time: > > ``` > file last modified on (DOS date/time): 1980 Jan 1 00:00:00 > file last modified on (UT extra field modtime): 1980 Jan 1 00:00:00 local > file last modified on (UT extra field modtime): 1980 Jan 1 08:00:00 UTC > ``` > > One second later `1980-01-01T00:00:01Z` works fine (considering the rounding > down to even seconds): > > ``` > file last modified on (DOS date/time): 1980 Jan 1 00:00:00 > ``` Thanks John. The 1980-01-01T00:00:00Z is a known issue because the zip xdostime format uses that value as the "marker" for date before-1980. I can adjust the --date value validation to be > 1980-01-01T00:00:00Z ? - PR: https://git.openjdk.java.net/jdk/pull/6481
Re: JDK-8275509: (jlink) SystemModulesPlugin generates a jdk.internal.module.SystemModules$all.class which isn't reproducible
On 11/7/21 9:44 PM, Jaikiran Pai wrote:
Hello Ioi,
On 02/11/21 12:13 am, Ioi Lam wrote:
Hi Jaikiran,
Thanks for writing the test case to explore the problems in this area.
Please see my comments below:
...
Generally speaking, CDS has two levels of archiving:
[1] archiving class metadata -- classes in the
$JAVA_HOME/lib/classlist are considered to be frequently loaded
classes. They are parsed from classfiles and stored into the CDS
archive. At run time, instead of parsing the classes from classfiles,
the VM directly use the pre-parsed version of these classes (as
InstanceKlass* in C++).
At runtime, all such pre-parsed classes are initially in the "loaded"
state. This means their static constructors will be executed when
these classes are referenced for the first time. So as far as Java
semantic is concerned, there's no difference between a pre-parsed
class vs a class loaded from classfile.
E.g, the examples of loggers in static initializers will be executed
at runtime.
[2] archiving heap objects
As shown in your test, we cannot arbitrarily archive the static
fields that were initialized during -Xshare:dump, because they may
have environment dependency.
The strategy used by CDS is to archive only a few static fields in a
small number of carefully hand-picked system classes. You can see the
list in
https://urldefense.com/v3/__https://github.com/openjdk/jdk/blob/977154400be786c500f36ba14188bff79db57075/src/hotspot/share/cds/heapShared.cpp*L97__;Iw!!ACWV5N9M2RV99hQ!eWsSVUa8qjZ0BWlbOonNdDtE7dcU3w4c9Su5hb24IXirxZFdPoS6wVBMi-78hA$
Thank you for that link. That helped. So essentially even though the
list of classes used for archiving class metadata isn't very tightly
controlled, the list of objects which are archived in the heap is much
more selective.
The reason why my PoC ended up reproducing this issue is because it
just so happened that I selected a class (ModuleDescriptor) which
(indirectly) is hand-picked in that list of classes that can end up in
the archived heap.
These static fields are stored into the CDS archive. At run time,
these fields are essentially copied into the Java heap, and then
picked up by code like this:
https://urldefense.com/v3/__https://github.com/openjdk/jdk/blob/977154400be786c500f36ba14188bff79db57075/src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java*L163__;Iw!!ACWV5N9M2RV99hQ!eWsSVUa8qjZ0BWlbOonNdDtE7dcU3w4c9Su5hb24IXirxZFdPoS6wVABkO6Q0w$
public static ModuleLayer boot() {
Counters.start();
ModuleLayer bootLayer;
ArchivedBootLayer archivedBootLayer = ArchivedBootLayer.get();
if (archivedBootLayer != null) {
assert canUseArchivedBootLayer();
bootLayer = archivedBootLayer.bootLayer();
BootLoader.getUnnamedModule(); // trigger of
BootLoader.
CDS.defineArchivedModules(ClassLoaders.platformClassLoader(),
ClassLoaders.appClassLoader());
// assume boot layer has at least one module providing a
service
// that is mapped to the application class loader.
JLA.bindToLoader(bootLayer, ClassLoaders.appClassLoader());
} else {
bootLayer = boot2();
}
In the case of the module graph, we remove things that depend on the
environment (such as CLASSPATH)
https://urldefense.com/v3/__https://github.com/openjdk/jdk/blob/977154400be786c500f36ba14188bff79db57075/src/hotspot/share/cds/heapShared.cpp*L190__;Iw!!ACWV5N9M2RV99hQ!eWsSVUa8qjZ0BWlbOonNdDtE7dcU3w4c9Su5hb24IXirxZFdPoS6wVAx46l0Bg$
The remaining parts of the archived module graph only depend on the
following system properties:
private static boolean canUseArchivedBootLayer() {
return getProperty("jdk.module.upgrade.path") == null &&
getProperty("jdk.module.path") == null &&
getProperty("jdk.module.patch.0") == null && //
--patch-module
getProperty("jdk.module.main") == null && //
--module
getProperty("jdk.module.addmods.0") == null && //
--add-modules
getProperty("jdk.module.limitmods") == null && //
--limit-modules
getProperty("jdk.module.addreads.0") == null && //
--add-reads
getProperty("jdk.module.addexports.0") == null && //
--add-exports
getProperty("jdk.module.addopens.0") == null; //
--add-opens
}
As a result, we will invalidate the archived module graph if these
properties differ between dump time and run time. The Java code above
only asserts that the check has already been done. The actual check
is done in here:
https://urldefense.com/v3/__https://github.com/openjdk/jdk/blob/977154400be786c500f36ba14188bff79db57075/src/hotspot/share/runtime/arguments.cpp*L1339__;Iw!!ACWV5N9M2RV99hQ!eWsSVUa8qjZ0BWlbOonNdDtE7dcU3w4c9Su5hb24IXirxZFdPoS6wVAwq_becQ$
Understood.
Am I misunderstanding the severity of this issue or is this serious
enough
Re: RFR: JDK-8276837: [macos]: Error when signing the additional launcher
On Wed, 1 Dec 2021 14:25:52 GMT, Andy Herrick wrote: > Before signing, unsign all executables and libraries (not just those in > runtime). > Also, run individual file sign and unsign commands in quiet mode. > Also, add test case to SigningAppImageTest to test that signing app that has > additional launcher works, and results in validly signed launcher and > additional launcher (original reported problem). last push was meant to be only one file - but got lots of unrelated changes. will close this one and open new PR. - PR: https://git.openjdk.java.net/jdk/pull/6636
Withdrawn: JDK-8276837: [macos]: Error when signing the additional launcher
On Wed, 1 Dec 2021 14:25:52 GMT, Andy Herrick wrote: > Before signing, unsign all executables and libraries (not just those in > runtime). > Also, run individual file sign and unsign commands in quiet mode. > Also, add test case to SigningAppImageTest to test that signing app that has > additional launcher works, and results in validly signed launcher and > additional launcher (original reported problem). This pull request has been closed without being integrated. - PR: https://git.openjdk.java.net/jdk/pull/6636
Re: RFR: JDK-8276837: [macos]: Error when signing the additional launcher [v2]
> Before signing, unsign all executables and libraries (not just those in > runtime). > Also, run individual file sign and unsign commands in quiet mode. > Also, add test case to SigningAppImageTest to test that signing app that has > additional launcher works, and results in validly signed launcher and > additional launcher (original reported problem). Andy Herrick has updated the pull request incrementally with 85 additional commits since the last revision: - JDK-8276837: [macos]: Error when signing the additional launcher - 8277797: Remove undefined/unused SharedRuntime::trampoline_size() Reviewed-by: dholmes, stuefe - 8277789: G1: G1CardSetConfiguration prefixes num_ and max_ used interchangeably Reviewed-by: mli, tschatzl - 8277878: Fix compiler tests after JDK-8275908 Reviewed-by: thartmann, chagedorn - 8277904: G1: Remove G1CardSetArray::max_entries Reviewed-by: tschatzl - 8277896: Remove unused BOTConstants member methods Reviewed-by: kbarrett - 8277450: Record number of references into collection set during gc Reviewed-by: kbarrett, iwalulya - 8277928: Fix compilation on macosx-aarch64 after 8276108 Reviewed-by: shade, dholmes - 8275241: Unused ArrayList is created in RequestEngine.addHooks Reviewed-by: egahlin - 8276108: Wrong instruction generation in aarch64 backend Co-authored-by: Nick Gasson Reviewed-by: aph, neliasso - ... and 75 more: https://git.openjdk.java.net/jdk/compare/34692c5d...2a3e19ff - Changes: - all: https://git.openjdk.java.net/jdk/pull/6636/files - new: https://git.openjdk.java.net/jdk/pull/6636/files/34692c5d..2a3e19ff Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=6636=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6636=00-01 Stats: 23719 lines in 463 files changed: 13755 ins; 6525 del; 3439 mod Patch: https://git.openjdk.java.net/jdk/pull/6636.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6636/head:pull/6636 PR: https://git.openjdk.java.net/jdk/pull/6636
Re: RFR: 8277072: ObjectStreamClass caches keep ClassLoaders alive [v4]
On Wed, 1 Dec 2021 12:18:05 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:
>
> Remove unused EntryFuture inner class from ObjectSteamClass
Without the use of SoftReference, memory pressure won't release any of the
cached info.
That seems to swing the other way from overly aggressively freeing memory with
the WeakReference (and needing to recompute) as the change originally proposed.
Its hard to tell in what environments it might be observed.
src/java.base/share/classes/java/io/ObjectStreamClass.java line 2133:
> 2131: if (oldReflector != null) {
> 2132: reflector = oldReflector;
> 2133: }
Map.computeIfAbsent(key, () -> new FieldReflector(matchFields, localDesc));
might be more compact.
test/jdk/java/io/ObjectStreamClass/TestOSCClassLoaderLeak.java line 52:
> 50: Class loadClass =
> myOwnClassLoader.loadClass("ObjectStreamClass_MemoryLeakExample");
> 51: Constructor con = loadClass.getConstructor();
> 52: con.setAccessible(true);
Isn't the constructor already public?
-
PR: https://git.openjdk.java.net/jdk/pull/6375
RFR: 8275731: CDS archived enums objects are recreated at runtime
**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.
-
Commit messages:
- 8275731: CDS archived enums objects are recreated at runtime
Changes: https://git.openjdk.java.net/jdk/pull/6653/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6653=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8275731
Stats: 829 lines in 16 files changed: 787 ins; 2 del; 40 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: JDK-8276837: [macos]: Error when signing the additional launcher
On Wed, 1 Dec 2021 19:59:57 GMT, Alexey Semenyuk wrote: >> Before signing, unsign all executables and libraries (not just those in >> runtime). >> Also, run individual file sign and unsign commands in quiet mode. >> Also, add test case to SigningAppImageTest to test that signing app that has >> additional launcher works, and results in validly signed launcher and >> additional launcher (original reported problem). > > test/jdk/tools/jpackage/macosx/SigningAppImageTest.java line 95: > >> 93: SigningBase.verifyCodesign(launcherPath, true); >> 94: SigningBase.verifyCodesign(testALPath, true); >> 95: } > > Would it make sense to extend the existing test with additional launcher > checks instead of creating another one? Packaging of a full JDK with signing > probably takes a while yes - that would be simpler - will do. - PR: https://git.openjdk.java.net/jdk/pull/6636
Re: RFR: JDK-8276837: [macos]: Error when signing the additional launcher
On Wed, 1 Dec 2021 14:25:52 GMT, Andy Herrick wrote: > Before signing, unsign all executables and libraries (not just those in > runtime). > Also, run individual file sign and unsign commands in quiet mode. > Also, add test case to SigningAppImageTest to test that signing app that has > additional launcher works, and results in validly signed launcher and > additional launcher (original reported problem). Changes requested by asemenyuk (Reviewer). test/jdk/tools/jpackage/macosx/SigningAppImageTest.java line 95: > 93: SigningBase.verifyCodesign(launcherPath, true); > 94: SigningBase.verifyCodesign(testALPath, true); > 95: } Would it make sense to extend the existing test with additional launcher checks instead of creating another one? Packaging of a full JDK with signing probably takes a while - PR: https://git.openjdk.java.net/jdk/pull/6636
RFR: JDK-8276837: [macos]: Error when signing the additional launcher
Before signing, unsign all executables and libraries (not just those in runtime). Also, run individual file sign and unsign commands in quiet mode. Also, add test case to SigningAppImageTest to test that signing app that has additional launcher works, and results in validly signed launcher and additional launcher (original reported problem). - Commit messages: - JDK-8276837: [macos]: Errorwhen signing the additional launcher Changes: https://git.openjdk.java.net/jdk/pull/6636/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6636=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8276837 Stats: 64 lines in 3 files changed: 33 ins; 5 del; 26 mod Patch: https://git.openjdk.java.net/jdk/pull/6636.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6636/head:pull/6636 PR: https://git.openjdk.java.net/jdk/pull/6636
Re: RFR: JDK-8276447 Deprecate finalization-related methods for removal [v2]
> Here are the code changes for the "Deprecate finalizers in the standard Java
> API" portion of JEP 421 ("Deprecate Finalization for Removal") for code
> review.
>
> This change makes the indicated deprecations, and updates the API spec for
> JEP 421. It also updates the relevant @SuppressWarning annotations.
>
> The CSR has been approved.
> An automated test build+test run passes cleanly (FWIW :D ).
Brent Christian has updated the pull request with a new target base due to a
merge or a rebase. The pull request now contains 32 commits:
- Merge branch 'master' into 8276447
- merge
- @SuppressWarnings(removal) in Windows CKey.java
- Add jls tag to runFinalization methods
- Update wording of Object.finalize, new paragraph is now bold
- update Object.finalize javadoc
- update Object.finalize JavaDoc and @deprecated tag
- Update Object.finalize deprecation message
- Indicate that runFinalizers does nothing if finalization is disabled or
removed
- Fix @since on @Deprecated for java.lang.Enum
- ... and 22 more: https://git.openjdk.java.net/jdk/compare/a363b7b9...e4986a48
-
Changes: https://git.openjdk.java.net/jdk/pull/6465/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6465=01
Stats: 194 lines in 62 files changed: 54 ins; 38 del; 102 mod
Patch: https://git.openjdk.java.net/jdk/pull/6465.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/6465/head:pull/6465
PR: https://git.openjdk.java.net/jdk/pull/6465
Re: RFR: JDK-8276447 Deprecate finalization-related methods for removal [v3]
> Here are the code changes for the "Deprecate finalizers in the standard Java
> API" portion of JEP 421 ("Deprecate Finalization for Removal") for code
> review.
>
> This change makes the indicated deprecations, and updates the API spec for
> JEP 421. It also updates the relevant @SuppressWarning annotations.
>
> The CSR has been approved.
> An automated test build+test run passes cleanly (FWIW :D ).
Brent Christian has updated the pull request with a new target base due to a
merge or a rebase. The pull request now contains 33 commits:
- Merge branch 'master' into 8276447
- Merge branch 'master' into 8276447
- merge
- @SuppressWarnings(removal) in Windows CKey.java
- Add jls tag to runFinalization methods
- Update wording of Object.finalize, new paragraph is now bold
- update Object.finalize javadoc
- update Object.finalize JavaDoc and @deprecated tag
- Update Object.finalize deprecation message
- Indicate that runFinalizers does nothing if finalization is disabled or
removed
- ... and 23 more: https://git.openjdk.java.net/jdk/compare/0dfb3a70...8cde0674
-
Changes: https://git.openjdk.java.net/jdk/pull/6465/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6465=02
Stats: 194 lines in 62 files changed: 54 ins; 38 del; 102 mod
Patch: https://git.openjdk.java.net/jdk/pull/6465.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/6465/head:pull/6465
PR: https://git.openjdk.java.net/jdk/pull/6465
Re: RFR: 8278080: Add --with-cacerts-src='user cacerts folder' to enable determinsitic cacerts generation
On Wed, 1 Dec 2021 18:30:06 GMT, Andrew Leonard wrote: > Addition of a configure option --with-cacerts-src='user cacerts folder' to > allow developers to specify their own cacerts PEM folder for generation of > the cacerts store using the deterministic openjdk GenerateCacerts tool. > > Signed-off-by: Andrew Leonard make/autoconf/jdk-options.m4 line 176: > 174: [specify alternative cacerts source folder containing > certificates])]) > 175: AC_MSG_CHECKING([for cacerts source]) > 176: if test "x$with_cacerts_src" == x; then Before this if block, please assign an empty value to CACERTS_SRC. Otherwise, if the user happens to have that variable set in the environment, that value will get recorded by configure, which is usually not something we want. - PR: https://git.openjdk.java.net/jdk/pull/6647
Re: RFR: 8278071: typos in MemorySegment::set, MemorySegment::setAtIndex javadoc
On Wed, 1 Dec 2021 13:49:42 GMT, Maurizio Cimadamore wrote: > There are some truncated statements in > MemorySegment::get/MemorySegment::getAtIndex javadoc, more specifically in > the `@throws` for `IndexOutOfBoundsException`. Trivial fix for an apparent cut-and-paste error. - Marked as reviewed by iris (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6635
RFR: 8278080: Add --with-cacerts-src='user cacerts folder' to enable determinsitic cacerts generation
Addition of a configure option --with-cacerts-src='user cacerts folder' to allow developers to specify their own cacerts PEM folder for generation of the cacerts store using the deterministic openjdk GenerateCacerts tool. Signed-off-by: Andrew Leonard - Commit messages: - 8278080: Add --with-cacerts-src='user cacerts folder' to enable determinsitic cacerts generation - 8278080: Add --with-cacerts-src='user cacerts folder' to enable determinsitic cacerts generation Changes: https://git.openjdk.java.net/jdk/pull/6647/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6647=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8278080 Stats: 21 lines in 3 files changed: 21 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/6647.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6647/head:pull/6647 PR: https://git.openjdk.java.net/jdk/pull/6647
RFR: 8278087: Redesign exceptions thrown for invalid jdk.serialFilter and jdk.serialFilterFactory.
The effects of invalid values of `jdk.serialFilter` and `jdk.serialFilterFactory` properties are incompletely specified. The behavior for invalid values of the properties is different and use an unconventional exception type, `ExceptionInInitializerError` and leave the `OIF.Config` class uninitialized. The exceptions in the `ObjectInputFilter.Config` class initialization caused by invalid values of the two properties, either by system properties supplied on the command line or security properties are logged. The `Config` class marks either or both the filter and filter factory values as unusable and remembers the exception message. Subsequent calls to the methods that get or set the filter or filter factory or create an `ObjectInputStream` throw `java.lang.IllegalStateException` with the remembered exception message. Constructing an `ObjectInputStream` calls both `Config.getSerialFilter` and `Config.getSerialFilterFactory`. The nature of the invalid property is reported as an `IllegalStateException` on first use. This PR supercedes https://github.com/openjdk/jdk/pull/6508 Document that setting an invalid property jdk.serialFilter disables deserialization - Commit messages: - 8278087: Redesign exceptions thrown for invalid jdk.serialFilter and jdk.serialFilterFactory. Changes: https://git.openjdk.java.net/jdk/pull/6645/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6645=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8278087 Stats: 218 lines in 4 files changed: 137 ins; 15 del; 66 mod Patch: https://git.openjdk.java.net/jdk/pull/6645.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6645/head:pull/6645 PR: https://git.openjdk.java.net/jdk/pull/6645
Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v14]
On Wed, 1 Dec 2021 18:02:05 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: > > Update src/jdk.jlink/share/classes/jdk/tools/jmod/JmodOutputStream.java > > Co-authored-by: Magnus Ihse Bursie I'm testing it now. So far, it's working great! I found one boundary problem. The magic instant of `1980-01-01T00:00:00Z` triggers an extended header with the wrong time: file last modified on (DOS date/time): 1980 Jan 1 00:00:00 file last modified on (UT extra field modtime): 1980 Jan 1 00:00:00 local file last modified on (UT extra field modtime): 1980 Jan 1 08:00:00 UTC One second later `1980-01-01T00:00:01Z` works fine (considering the rounding down to even seconds): file last modified on (DOS date/time): 1980 Jan 1 00:00:00 - PR: https://git.openjdk.java.net/jdk/pull/6481
Re: RFR: 8277322: Document that setting an invalid property jdk.serialFilter disables deserialization
On Tue, 30 Nov 2021 20:43:23 GMT, Roger Riggs wrote: >> This is about the second line of defense; what happens when the developer >> deliberately ignores the first error. >> If the command line parameters are invalid it might be an option to call >> `System.exit(1)` but there >> is no precedent for that and it seems undesirable. >> >> Any and all deserialization is only possible after the command line or >> security properties, if any, are successfully applied. >> In the examples above, the constructors for `ObjectInputStream` do not >> succeed if either the serial filter or filter factory are not valid. The >> builtin filter factory applies the default filter regardless of the setting >> of an `ObjectInputFilter` set on the stream. The only way to completely >> control the filter combinations is to provide >> a valid filter factory on the command line; but that is not the case raising >> the issue here. >> >> The initialization of both could be re-specified and re-implemented to allow >> the initialization of `Config` to >> complete successfully but defer throwing an exception (or Error) until >> either filter or filter factory >> was requested from `Config.getSerialFilter` or >> `Config.getSerialFilterFactory`. >> Both of them are called from the `ObjectInputStream` constructors. >> At present, it is incompletely specified and implemented to throw >> `IllegalStateException` for `getSerialFilterFactory`. >> >> The `NCDFE` is a more reliable backstop against misuse from any source, >> including reflection, than the alternative. > > The use of `ExceptionInInitializerError` can be completely replaced for > invalid values of `jdk.serialFilter` and `jdk.serialFilterFactory` with: > > - If either property is supplied and is invalid; deserialization is disabled > by: > - `OIF.Config.getSerialFilter()` and `OIF.Config.setSerialFilter(...)` throw > IllegalStateException with the exception message thrown from > `Config.createFilter(pattern)` > - `OIF.Config.getSerialFilterFactory()` and > `OIF.Config.setSerialFilterFactory(...)` throw IllegalStateException with the > exception message from the attempt to construct the filter factory. > - Both `new ObjectInputStream(...)` constructors call both > `OIF.Config.getSerialFilter()` and `OIF.Config.getSerialFilterFactory()` and > so will throw the appropriate `IllegalStateException` for invalid values of > the properties. > - The static initialization of `OIF.Config` does not throw any exceptions (so > no `ExceptionInInitializerError`) but hold the state so that the method above > can throw `IllegalStateException` with the informative message. > - The `IllegalStateException`s will be thrown just slightly later than > previously, now after the `Config` class is initialized instead of during > initialization. > - The javadoc of the `Config` class will replace the descriptions of the > previous error with descriptions of `ISE` and each of the methods mentioned > above will have an added `IllegalStateException` documented referring to the > property values. > > Its not strictly compatible with the previous behavior but occurs only in the > case of badly formed parameters on the command line. With the change in scope of the solution, a new PR has been created: https://github.com/openjdk/jdk/pull/6645 Please review that instead. - PR: https://git.openjdk.java.net/jdk/pull/6508
Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v14]
> 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: Update src/jdk.jlink/share/classes/jdk/tools/jmod/JmodOutputStream.java Co-authored-by: Magnus Ihse Bursie - Changes: - all: https://git.openjdk.java.net/jdk/pull/6481/files - new: https://git.openjdk.java.net/jdk/pull/6481/files/c84bc5ad..2f3a7fc2 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=6481=13 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6481=12-13 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 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: JDK-8273056 java.util.random does not correctly sample exponential or Gaussian distributions [v2]
On Wed, 1 Dec 2021 17:38:54 GMT, Jim Laskey wrote: >> The modified ziggurat algorithm is not correctly implemented in >> `java.base/jdk/internal/util/random/RandomSupport.java`. >> >> Create a histogram of a million samples using 2000 uniform bins with the >> following range: >> Exponential range from 0 to 12. Gaussian range from -8 to 8. >> >> This does not pass a Chi-square test. If you look at the histogram it is >> obviously not showing the shape of the PDF for these distributions. Look >> closely at the range around zero (e.g. +/- 0.5). > > Jim Laskey 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 two additional commits since > the last revision: > > - Merge branch 'master' into 8273056 > - 8273056 - java.util.random does not correctly sample exponential or > Gaussian distributions Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6353
Re: RFR: JDK-8273056 java.util.random does not correctly sample exponential or Gaussian distributions [v2]
> The modified ziggurat algorithm is not correctly implemented in > `java.base/jdk/internal/util/random/RandomSupport.java`. > > Create a histogram of a million samples using 2000 uniform bins with the > following range: > Exponential range from 0 to 12. Gaussian range from -8 to 8. > > This does not pass a Chi-square test. If you look at the histogram it is > obviously not showing the shape of the PDF for these distributions. Look > closely at the range around zero (e.g. +/- 0.5). Jim Laskey 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 two additional commits since the last revision: - Merge branch 'master' into 8273056 - 8273056 - java.util.random does not correctly sample exponential or Gaussian distributions - Changes: - all: https://git.openjdk.java.net/jdk/pull/6353/files - new: https://git.openjdk.java.net/jdk/pull/6353/files/b10c4793..b6679479 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=6353=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6353=00-01 Stats: 69228 lines in 1218 files changed: 46725 ins; 12712 del; 9791 mod Patch: https://git.openjdk.java.net/jdk/pull/6353.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6353/head:pull/6353 PR: https://git.openjdk.java.net/jdk/pull/6353
Re: RFR: JDK-8273056 java.util.random does not correctly sample exponential or Gaussian distributions
On Wed, 1 Dec 2021 14:21:51 GMT, Jim Laskey wrote: > > Should there be a regression test here? > > I can ask the submitter to provide a new test based on his testing (new JBS > issue.) The complexity of the test is beyond in house expertise. Okay. - PR: https://git.openjdk.java.net/jdk/pull/6353
Re: RFR: JDK-8273056 java.util.random does not correctly sample exponential or Gaussian distributions
On Thu, 11 Nov 2021 13:59:51 GMT, Jim Laskey wrote: > The modified ziggurat algorithm is not correctly implemented in > `java.base/jdk/internal/util/random/RandomSupport.java`. > > Create a histogram of a million samples using 2000 uniform bins with the > following range: > Exponential range from 0 to 12. Gaussian range from -8 to 8. > > This does not pass a Chi-square test. If you look at the histogram it is > obviously not showing the shape of the PDF for these distributions. Look > closely at the range around zero (e.g. +/- 0.5). Marked as reviewed by darcy (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6353
Integrated: 8177819: DateTimeFormatterBuilder zone parsing should recognise DST
On Tue, 23 Nov 2021 17:00:58 GMT, Naoto Sato wrote: > This fix intends to honor the type (std/dst/generic) of parsed zone name for > selecting the offset at the overlap. Corresponding CSR has also been drafted. This pull request has now been integrated. Changeset: a363b7b9 Author:Naoto Sato URL: https://git.openjdk.java.net/jdk/commit/a363b7b9217cbb9a7580a87b812da8d5a4215326 Stats: 204 lines in 5 files changed: 74 ins; 83 del; 47 mod 8177819: DateTimeFormatterBuilder zone parsing should recognise DST 8277049: ZonedDateTime parse in Fall DST transition fails to retain the correct zonename. Reviewed-by: joehw, scolebourne - PR: https://git.openjdk.java.net/jdk/pull/6527
RFR: 8255266: 2021-11-27 public suffix list update v 3c213aa
Update Public Suffix List data to the latest version at https://github.com/publicsuffix/list. - Commit messages: - 8255266: 2021-11-27 public suffix list update v 3c213aa Changes: https://git.openjdk.java.net/jdk/pull/6643/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6643=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8255266 Stats: 1254 lines in 5 files changed: 887 ins; 215 del; 152 mod Patch: https://git.openjdk.java.net/jdk/pull/6643.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6643/head:pull/6643 PR: https://git.openjdk.java.net/jdk/pull/6643
Re: RFR: 8278028: [test-library] Warnings cleanup of the test library
On Wed, 1 Dec 2021 14:47:54 GMT, Roger Riggs wrote:
> Compilation warnings of the test library introduce noise in test output and
> should be addressed or suppressed.
> Changes include:
> - SuppressWarnings("deprecation") and SuppressWarnings("removal")
> - Adding type parameters to Raw types
> - Adding a hashCode method where equals was already present
Changes look reasonable to me but since I am not familiar with all those files
it might be best to get at least one additional reviewer.
-
Marked as reviewed by dfuchs (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/6638
Integrated: JDK-8278014: [vectorapi] Remove test run script
On Tue, 30 Nov 2021 19:22:53 GMT, Paul Sandoz wrote: > Remove Vector API scripts for building and running tests. `jtreg` should be > used instead. > > Also updated the test generation script to remove options that assume > mercurial as the code repository. This pull request has now been integrated. Changeset: 9b3e6720 Author:Paul Sandoz URL: https://git.openjdk.java.net/jdk/commit/9b3e67205913daa1960373a4ab33422137082696 Stats: 161 lines in 5 files changed: 0 ins; 156 del; 5 mod 8278014: [vectorapi] Remove test run script Reviewed-by: sviswanathan, jiefu, shade - PR: https://git.openjdk.java.net/jdk/pull/6621
Integrated: 8193682: Infinite loop in ZipOutputStream.close()
On Wed, 15 Sep 2021 07:40:35 GMT, Ravi Reddy wrote: > Hi all, > > Please review this fix for Infinite loop in ZipOutputStream.close(). > The main issue here is when ever there is an exception during close > operations on GZip we are not setting the deflator to a finished state which > is leading to an infinite loop when we try writing on the same GZip instance( > since we use while(!def.finished()) inside the write operation). > > Thanks, > Ravi This pull request has now been integrated. Changeset: 1e9ed54d Author:Ravi Reddy Committer: Sean Coffey URL: https://git.openjdk.java.net/jdk/commit/1e9ed54d362b8c57be5fbbac2de5afbd0f05435f Stats: 258 lines in 4 files changed: 191 ins; 29 del; 38 mod 8193682: Infinite loop in ZipOutputStream.close() Reviewed-by: lancea, coffeys - PR: https://git.openjdk.java.net/jdk/pull/5522
Re: RFR: 8193682: Infinite loop in ZipOutputStream.close() [v14]
On Wed, 1 Dec 2021 05:35:11 GMT, Ravi Reddy wrote: >> Hi all, >> >> Please review this fix for Infinite loop in ZipOutputStream.close(). >> The main issue here is when ever there is an exception during close >> operations on GZip we are not setting the deflator to a finished state which >> is leading to an infinite loop when we try writing on the same GZip >> instance( since we use while(!def.finished()) inside the write operation). >> >> Thanks, >> Ravi > > Ravi Reddy has updated the pull request incrementally with one additional > commit since the last revision: > > Change in test case name from GZipLoopTest.java to CloseDeflaterTest , > moved testcase to util/zip/ Marked as reviewed by coffeys (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5522
RFR: 8278028: [test-library] Warnings cleanup of the test library
Compilation warnings of the test library introduce noise in test output and
should be addressed or suppressed.
Changes include:
- SuppressWarnings("deprecation") and SuppressWarnings("removal")
- Adding type parameters to Raw types
- Adding a hashCode method where equals was already present
-
Commit messages:
- 8278028: [test-library] Warnings cleanup of the test library
Changes: https://git.openjdk.java.net/jdk/pull/6638/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6638=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8278028
Stats: 23 lines in 14 files changed: 10 ins; 1 del; 12 mod
Patch: https://git.openjdk.java.net/jdk/pull/6638.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/6638/head:pull/6638
PR: https://git.openjdk.java.net/jdk/pull/6638
RFR: 8278065: Refactor subclassAudits to use ClassValue
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: - [ ] tier1 - [ ] tier2 - [ ] tier3 - Depends on: https://git.openjdk.java.net/jdk/pull/6375 Commit messages: - 8278065: Refactor subclassAudits to use ClassValue Changes: https://git.openjdk.java.net/jdk/pull/6637/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6637=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8278065 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: JDK-8273056 java.util.random does not correctly sample exponential or Gaussian distributions
On Tue, 30 Nov 2021 22:23:01 GMT, Joe Darcy wrote: > Should there be a regression test here? I can ask the submitter to provide a new test based on his testing (new JBS issue.) The complexity of the test is beyond in house expertise. - PR: https://git.openjdk.java.net/jdk/pull/6353
Re: RFR: 8177819: DateTimeFormatterBuilder zone parsing should recognise DST [v4]
On Mon, 29 Nov 2021 17:41:34 GMT, Naoto Sato wrote: >> This fix intends to honor the type (std/dst/generic) of parsed zone name for >> selecting the offset at the overlap. Corresponding CSR has also been drafted. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Refined wording. Removed `LENIENT` parser. Added tests for CI. Marked as reviewed by scolebourne (Author). - PR: https://git.openjdk.java.net/jdk/pull/6527
Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v13]
On Wed, 1 Dec 2021 11:06:12 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 two additional > commits since the last revision: > > - 8276766: Enable jar and jmod to produce deterministic timestamped content > >Signed-off-by: Andrew Leonard > - 8276766: Enable jar and jmod to produce deterministic timestamped content > >Signed-off-by: Andrew Leonard src/jdk.jlink/share/classes/jdk/tools/jmod/JmodOutputStream.java line 65: > 63: > 64: private final ZipOutputStream zos; > 65: private final LocalDateTime date; Suggestion: private final LocalDateTime date; No need for extra spaces. - PR: https://git.openjdk.java.net/jdk/pull/6481
Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v13]
On Mon, 29 Nov 2021 17:07:52 GMT, Alan Bateman wrote: >> @andrew-m-leonard I see you've add several APIs to ZipEntry in this PR. I >> think this part will require discussion as it's not immediately clear that >> we should over burden the ZipEntry API as proposed. > >> @AlanBateman yes, see above comment, thanks > > This is a significant change to the ZipEntry API that will require discussion > and agreement. Can you start a discussion on core-libs-dev about the topic? > You could start with a summary of the problem and the API and non-API options > that have been explored. @AlanBateman @LanceAndersen @jgneff The new update based on our discussions is now ready for review please. I have also updated the CSR: https://bugs.openjdk.java.net/browse/JDK-8277755 which is review ready as well. thank you Andrew - PR: https://git.openjdk.java.net/jdk/pull/6481
RFR: 8278071: typos in MemorySegment::set, MemorySegment::setAtIndex javadoc
There are some truncated statements in MemorySegment::get/MemorySegment::getAtIndex javadoc, more specifically in the `@throws` for `IndexOutOfBoundsException`. - Commit messages: - Fix typos in MemorySegment dereference setters Changes: https://git.openjdk.java.net/jdk/pull/6635/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6635=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8278071 Stats: 16 lines in 1 file changed: 16 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/6635.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6635/head:pull/6635 PR: https://git.openjdk.java.net/jdk/pull/6635
Re: RFR: JDK-8276674 : Malformed Javadoc inline tags in JDK source in java/util/stream
On Thu, 18 Nov 2021 03:22:50 GMT, Tim Prinzing wrote: > JDK-8276674 : Malformed Javadoc inline tags in JDK source in java/util/stream Given that this PR affects files that aren't rooted under java/util/stream, I would either rename the PR or exclude the unrelated files. If this PR is going to sit there for some more time, I would prefer the latter. This is because the PR has already conflicted with a few other issues related to malformed javadoc tags (e.g. JDK-8276683 and JDK-8276684). So either rename and integrate soon, or exclude the unrelated files. - PR: https://git.openjdk.java.net/jdk/pull/6443
Re: RFR: 8277647: [REDO] JDK-8277507 Add jlink.debug system property while launching jpackage tests to help diagonize recent intermittent failures
On Wed, 24 Nov 2021 08:54:08 GMT, Jaikiran Pai wrote: > Can I please get a review of this change which adds `jlink.debug=true` system > property while launching `jpackage` tests? > > The previous fix for this in https://github.com/openjdk/jdk/pull/6491 didn't > take into account the part where the `jpackage` tool gets launched as a > `ToolProvider` from some of these tests. This resulted in a large number of > tests failing (across different OS) in `tier2` with errors like: > > > Error: Invalid Option: [-J-Djlink.debug=true] > > > In this current PR, the changed code now takes into account the possibility > of `jpackage` being launched as a `ToolProvider` and in such cases doesn't > add this option. To achieve this, the adding of this argument is delayed till > when the actual execution is about to happen and thus it's now done in the > `adjustArgumentsBeforeExecution()` method of the jpackage test framework. > > With this change, I have now run the `jdk:tier2` locally on a macos instance > and the tests have all passed: > > > Test results: passed: 3,821; failed: 3 > Report written to > jdk/build/macosx-x86_64-server-release/test-results/jtreg_test_jdk_tier2/html/report.html > Results written to > jdk/build/macosx-x86_64-server-release/test-support/jtreg_test_jdk_tier2 > Error: Some tests failed or other problems occurred. > Finished running test 'jtreg:test/jdk:tier2' > Test report is stored in > build/macosx-x86_64-server-release/test-results/jtreg_test_jdk_tier2 > > == > Test summary > == >TEST TOTAL PASS FAIL ERROR > >>> jtreg:test/jdk:tier2 3824 3821 3 0 << > == > > The 3 failing tests are unrelated to this change and belong to the > `java/nio/channels/DatagramChannel/` test package. > Furthermore, I've looked into the generated logs of the following tests to > verify that the `-J-Djlink.debug=true` does get passed in relevant tests and > doesn't in those that failed previously in `tier2`: > > test/jdk/tools/jpackage/share/MultiLauncherTwoPhaseTest.java > test/jdk/tools/jpackage/macosx/NameWithSpaceTest.java > test/jdk/tools/jpackage/share/ArgumentsTest.java > > A sample from one of the logs where this system property is expected to be > passed along: > >> TRACE: exec: Execute >> [jdk/build/macosx-x86_64-server-release/images/jdk/bin/jpackage --input >> ./test/input --dest ./test/output --name "Name With Space" --type pkg >> --main-jar hello.jar --main-class Hello -J-Djlink.debug=true --verbose](15); >> inherit I/O... > > > I would still like to request someone with access to CI or other OSes (like > Windows and Linux) to help test `tier2` against this PR. LGTM - Marked as reviewed by sundar (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6534
Re: RFR: 8277072: ObjectStreamClass caches keep ClassLoaders alive [v3]
On Wed, 1 Dec 2021 10:49:54 GMT, Peter Levart wrote: > ...I think that you could remove now obsolete > java.io.ObjectStreamClass.EntryFuture nested class. It's not used any more. Done. > It would be nice to follow-up this patch with patches that make use of > ClassValue also for: > > * java.io.ObjectInputStream.Caches#subclassAudits > > * java.io.ObjectOutputStream.Caches#subclassAudits > > > ...this way the common static machinery like: > > * java.io.ObjectStreamClass#processQueue > > * java.io.ObjectStreamClass.WeakClassKey > ...could get removed as it is not used in ObjectStreamClass any more. I filed: https://bugs.openjdk.java.net/browse/JDK-8278065 Thanks! - PR: https://git.openjdk.java.net/jdk/pull/6375
Re: RFR: 8277072: ObjectStreamClass caches keep ClassLoaders alive [v4]
> 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: Remove unused EntryFuture inner class from ObjectSteamClass - Changes: - all: https://git.openjdk.java.net/jdk/pull/6375/files - new: https://git.openjdk.java.net/jdk/pull/6375/files/2ed745b3..b7d53adc Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=6375=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6375=02-03 Stats: 65 lines in 1 file changed: 0 ins; 65 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/6375.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6375/head:pull/6375 PR: https://git.openjdk.java.net/jdk/pull/6375
Re: RFR: 8277647: [REDO] JDK-8277507 Add jlink.debug system property while launching jpackage tests to help diagonize recent intermittent failures
On Wed, 24 Nov 2021 08:54:08 GMT, Jaikiran Pai wrote: > Can I please get a review of this change which adds `jlink.debug=true` system > property while launching `jpackage` tests? > > The previous fix for this in https://github.com/openjdk/jdk/pull/6491 didn't > take into account the part where the `jpackage` tool gets launched as a > `ToolProvider` from some of these tests. This resulted in a large number of > tests failing (across different OS) in `tier2` with errors like: > > > Error: Invalid Option: [-J-Djlink.debug=true] > > > In this current PR, the changed code now takes into account the possibility > of `jpackage` being launched as a `ToolProvider` and in such cases doesn't > add this option. To achieve this, the adding of this argument is delayed till > when the actual execution is about to happen and thus it's now done in the > `adjustArgumentsBeforeExecution()` method of the jpackage test framework. > > With this change, I have now run the `jdk:tier2` locally on a macos instance > and the tests have all passed: > > > Test results: passed: 3,821; failed: 3 > Report written to > jdk/build/macosx-x86_64-server-release/test-results/jtreg_test_jdk_tier2/html/report.html > Results written to > jdk/build/macosx-x86_64-server-release/test-support/jtreg_test_jdk_tier2 > Error: Some tests failed or other problems occurred. > Finished running test 'jtreg:test/jdk:tier2' > Test report is stored in > build/macosx-x86_64-server-release/test-results/jtreg_test_jdk_tier2 > > == > Test summary > == >TEST TOTAL PASS FAIL ERROR > >>> jtreg:test/jdk:tier2 3824 3821 3 0 << > == > > The 3 failing tests are unrelated to this change and belong to the > `java/nio/channels/DatagramChannel/` test package. > Furthermore, I've looked into the generated logs of the following tests to > verify that the `-J-Djlink.debug=true` does get passed in relevant tests and > doesn't in those that failed previously in `tier2`: > > test/jdk/tools/jpackage/share/MultiLauncherTwoPhaseTest.java > test/jdk/tools/jpackage/macosx/NameWithSpaceTest.java > test/jdk/tools/jpackage/share/ArgumentsTest.java > > A sample from one of the logs where this system property is expected to be > passed along: > >> TRACE: exec: Execute >> [jdk/build/macosx-x86_64-server-release/images/jdk/bin/jpackage --input >> ./test/input --dest ./test/output --name "Name With Space" --type pkg >> --main-jar hello.jar --main-class Hello -J-Djlink.debug=true --verbose](15); >> inherit I/O... > > > I would still like to request someone with access to CI or other OSes (like > Windows and Linux) to help test `tier2` against this PR. Any help reviewing this please? - PR: https://git.openjdk.java.net/jdk/pull/6534
Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v12]
On Wed, 1 Dec 2021 01:37:55 GMT, John Neffenger 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 > > I found it helpful to look at what others have done. > > The Debian `strip-nondeterminism` tool clamps the low end of the date to > `1980-01-01T12:01:00`. All other dates are permitted no matter the > consequence. That tool is written in Perl and is able to access all of the > timestamp fields individually. See the [`zip.pm` file][1] for details. > > Gradle decided that the only permissible date is `1980-02-01T00:00:00` in the > default time zone of the JVM. Period. End of story. No customization at all. > [They use a trick][2] with the old `GregorianCalendar` class to counteract > the effect in `ZipEntry.setTime` of the default time zone. They use the > `org.apache.tools.zip.ZipEntry` subclass of `java.util.zip.ZipEntry`, but > many methods pass through to the super class. See the [`ZipCopyAction.java` > file][3] for details. > > The crux of our problem, as Andrew made more clear to me, is that the methods > of `ZipEntry` do not allow for independent access to the two fields: the "DOS > date/time" field and the "UT extra field modtime". When trying to set either > one of them, the `ZipEntry` class can overwrite the other using the default > time zone of the JVM. > > @andrew-m-leonard So I agree. Either document that the archives are > reproducible only within a certain range of dates, or disallow the dates that > are out of range. Then we should be good for at least the next 78 years. > > [1]: > https://salsa.debian.org/reproducible-builds/strip-nondeterminism/-/blob/master/lib/File/StripNondeterminism/handlers/zip.pm#L40 > [2]: https://github.com/gradle/gradle/issues/12895#issuecomment-618850095 > [3]: > https://github.com/gradle/gradle/blob/master/subprojects/core/src/main/java/org/gradle/api/internal/file/archive/ZipCopyAction.java#L42-L56 @jgneff thank you John for you test program, i've been using your previous version which is similar. So please can you review the latest commit, it basically follows the same method of your test program using a ZonedDateTime instant from the input --date converted to a LocalDateTime of the same Instant in UTC, and then passed to e.setTimeLocal(ldt) It also only allows --date in the local date range of 1980->2099 Here's a manual test from 2 timezones of this latest commit: ~/workspace/repbuild$ sudo timedatectl set-timezone GMT ~/workspace/repbuild$ jar --create --date="2021-01-06T14:36:00+02:00" --file=myjar1.jar Time.* ~/workspace/repbuild$ sudo timedatectl set-timezone EST ~/workspace/repbuild$ jar --create --date="2021-01-06T14:36:00+02:00" --file=myjar2.jar Time.* ~/workspace/repbuild$ zipinfo -v myjar1.jar | grep -e Archive -e modified Archive: myjar1.jar file last modified on (DOS date/time): 2021 Jan 6 12:36:00 file last modified on (DOS date/time): 2021 Jan 6 12:36:00 file last modified on (DOS date/time): 2021 Jan 6 12:36:00 file last modified on (DOS date/time): 2021 Jan 6 12:36:00 ~/workspace/repbuild$ zipinfo -v myjar2.jar | grep -e Archive -e modified Archive: myjar2.jar file last modified on (DOS date/time): 2021 Jan 6 12:36:00 file last modified on (DOS date/time): 2021 Jan 6 12:36:00 file last modified on (DOS date/time): 2021 Jan 6 12:36:00 file last modified on (DOS date/time): 2021 Jan 6 12:36:00 I have also added reproducible jar tests across timezones. thanks - PR: https://git.openjdk.java.net/jdk/pull/6481
Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v13]
> 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 two additional commits since the last revision: - 8276766: Enable jar and jmod to produce deterministic timestamped content Signed-off-by: Andrew Leonard - 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/283e435a..c84bc5ad Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=6481=12 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6481=11-12 Stats: 58 lines in 1 file changed: 50 ins; 0 del; 8 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: 8277072: ObjectStreamClass caches keep ClassLoaders alive [v3]
On Tue, 30 Nov 2021 11:25:48 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 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 six additional > commits since the last revision: > > - 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 ...I think that you could remove now obsolete java.io.ObjectStreamClass.EntryFuture nested class. It's not used any more. It would be nice to follow-up this patch with patches that make use of ClassValue also for: - java.io.ObjectInputStream.Caches#subclassAudits - java.io.ObjectOutputStream.Caches#subclassAudits ...this way the common static machinery like: - java.io.ObjectStreamClass#processQueue - java.io.ObjectStreamClass.WeakClassKey ...could get removed as it is not used in ObjectStreamClass any more. - PR: https://git.openjdk.java.net/jdk/pull/6375
Integrated: 8277861: Terminally deprecate Thread.stop
On Tue, 30 Nov 2021 14:52:37 GMT, Alan Bateman wrote: > Thread.stop is inherently unsafe and has been deprecated since Java 1.2 > (1998). It's time to terminally deprecate this method so it can be degraded > and removed in the future. > > This PR does not propose any changes to the JVM TI StopThread function (or > the corresponding JDWP command or JDI method). This pull request has now been integrated. Changeset: fde0b95e Author:Alan Bateman URL: https://git.openjdk.java.net/jdk/commit/fde0b95ede68c188479852c46df7e28dc4b79594 Stats: 2 lines in 2 files changed: 1 ins; 0 del; 1 mod 8277861: Terminally deprecate Thread.stop Reviewed-by: rriggs, mchung, uschindler, dholmes - PR: https://git.openjdk.java.net/jdk/pull/6616
Re: RFR: JDK-8278014: [vectorapi] Remove test run script [v2]
On Tue, 30 Nov 2021 23:35:08 GMT, Paul Sandoz wrote: >> Remove Vector API scripts for building and running tests. `jtreg` should be >> used instead. >> >> Also updated the test generation script to remove options that assume >> mercurial as the code repository. > > Paul Sandoz has updated the pull request incrementally with one additional > commit since the last revision: > > Update copywrite year. Marked as reviewed by shade (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6621
Re: RFR: 8277992: Add fast jdk_svc subtests to jdk:tier3
On Wed, 1 Dec 2021 02:29:14 GMT, David Holmes wrote: > Hi @shipilev , We need to have someone look at the impact of this on our CI. > I don't think we run the tier4 group as defined in our tier 4 so we may not > see those execution time savings that offset the extra cost to tier3. OK, I can wait for those who have more insight in Oracle testing pipelines check their workflows with this change. I have no insight in Oracle infra, so somebody else have to do it. Now that Igor left, who should we be talking to? - PR: https://git.openjdk.java.net/jdk/pull/6619
