November 2021 - core-libs-dev

RFR: 8277861: Terminally deprecate Thread.stop
by Alan Bateman 02 Dec '21

02 Dec '21

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). ------------- Commit messages: - Initial commit Changes: https://git.openjdk.java.net/jdk/pull/6616/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6616&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8277861 Stats: 2 lines in 2 files changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/6616.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6616/head:pull/6616 PR: https://git.openjdk.java.net/jdk/pull/6616

9 11

RFR: 8277647: [REDO] JDK-8277507 Add jlink.debug system property while launching jpackage tests to help diagonize recent intermittent failures
by Jaikiran Pai 02 Dec '21

02 Dec '21

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. ------------- Commit messages: - 8277647: [REDO] JDK-8277507 Add jlink.debug system property while launching jpackage tests to help diagonize recent intermittent failures Changes: https://git.openjdk.java.net/jdk/pull/6534/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6534&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8277647 Stats: 5 lines in 1 file changed: 5 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/6534.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6534/head:pull/6534 PR: https://git.openjdk.java.net/jdk/pull/6534

6 9

JDK-8275509: (jlink) SystemModulesPlugin generates a jdk.internal.module.SystemModules$all.class which isn't reproducible
by Jaikiran Pai 01 Dec '21

01 Dec '21

This relates to the intermittent failures in tools/jlink/JLinkReproducibleTest.java test case which has been ProblemListed for a while now. The root cause is https://bugs.openjdk.java.net/browse/JDK-8275509. I couldn't find any specific mailing lists for jlink tool and I remember seeing jlink/jpackage related discussions on this mailing list previously, so creating this discussion here. The jlink tool uses plugins to let them transform contents of the modules that are part of the image. One such plugin is the SystemModulesPlugin and as per its javadoc specification its role is to prevent parsing of module-info.class files during JVM startup. To do so, it creates specialized dynamically generated class files (during jlink). The class file(s) are then bundled into the image that gets generated. One such class file is a dynamically generated jdk/internal/module/SystemModules$all.class. The SystemModulesPlugin generates the bytecode for this class. The details of that bytecode are very implementation specific. One part of the bytecode generation involves bytecode for an internal method called "moduleDescriptors()". One of the statements generated for this method implementation is a call to jdk.internal.module.Builder.build(int hashCode) method[1]. What this call does is, it creates a (pre-populated/validated) instance of the java.lang.module.ModuleDescriptor. The SystemModulesPlugin, when generating the bytecode of this method, uses the hashCode() of the ModuleDescriptor of the current JVM instance (through a typical ModuleDescriptor#hashCode() call)[2]. By doing so, it ends up generating bytecode which "embeds" the current runtime specific hashcode into the generated class file. The contract of java.lang.Object#hashCode() states: " ... This integer need not remain consistent from one execution of an application to another execution of the same application. .... " Effectively, what this means is, if jlink is used to generate an image with the exact same set of modules (requirements, package, exports, opens etc...) it still can (and does) end up generating a jdk/internal/module/SystemModules$all.class file whose binary content will differ across these runs, thus being non-reproducible. The implementation in java.lang.module.ModuleDescriptor is such that if the hashcode passed to it (during construction) is 0, then it lazily computes the correct hashcode whenever the invocation of hashCode() happens at runtime. What that means is, the SystemModulesPlugin in its bytecode generation for the moduleDescriptors() method could always pass 0 as the hashcode to the jdk.internal.module.Builder.build(int hashCode) call. That's something that I experimented with and after that change, with 100s of runs of the JLinkReproducibleTest, I no longer get any failures. However, given that the SystemModulesPlugin's goal appears to be to reduce the booting time for system modules, I was wondering if this change would introduce any performance penalty that is big enough. What this change will end up doing is, whenever the next time the hashCode method on instances of the system module's ModuleDescriptor gets called, it will have to compute it and that computation is relatively expensive (given how many "components" it uses to calculate it[3]). It's a (mostly) one time thing for each instance, but I don't know how expensive that would be. Do we have any existing benchmarks in this area that I could reuse to see what performance impact this might have? Keeping aside the performance issue for a bit, is this proposed patch something worth considering: --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java @@ -1099,7 +1099,13 @@ public final class SystemModulesPlugin extends AbstractPlugin { mv.visitVarInsn(ALOAD, MD_VAR); pushInt(mv, index); mv.visitVarInsn(ALOAD, BUILDER_VAR); - mv.visitLdcInsn(md.hashCode()); + // Let the ModuleDescriptor hashcode be computed at runtime. + // Embedding the current hashcode of the ModuleDescriptor + // into the bytecode of a generated class can cause the generated + // bytecode to be not reproducible, since an object's hashcode is allowed + // to change across JVM runs. + mv.visitLdcInsn(0); // the hashcode to be passed to the + // jdk.internal.module.Builder.build(int) method mv.visitMethodInsn(INVOKEVIRTUAL, MODULE_DESCRIPTOR_BUILDER, "build", "(I)Ljava/lang/module/ModuleDescriptor;", false); The other option I experimented with was to make ModuleDescriptor#hashCode() generate the same hashcode across multiple JVM runs. Although I do have a "working" version of that change, I decided not to spend too much time on it because the java.lang.Object#hashCode() contract itself clearly states that this value isn't expected to be same across multiple JVM runs. So whatever I do here is going to be brittle. [1] https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/jdk/… [2] https://github.com/openjdk/jdk/blob/master/src/jdk.jlink/share/classes/jdk/… [3] https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java… P.S: The major part of accurate investigation work for these intermittent failures was already done by Dongbo He in https://bugs.openjdk.java.net/browse/JDK-8258945. I just used that script and details to try and come up with a patch. -Jaikiran

4 15

RFR: 8177819: DateTimeFormatterBuilder zone parsing should recognise DST
by Naoto Sato 01 Dec '21

01 Dec '21

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. ------------- Commit messages: - 8177819: DateTimeFormatterBuilder zone parsing should recognise DST Changes: https://git.openjdk.java.net/jdk/pull/6527/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6527&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8177819 Stats: 112 lines in 5 files changed: 61 ins; 0 del; 51 mod Patch: https://git.openjdk.java.net/jdk/pull/6527.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6527/head:pull/6527 PR: https://git.openjdk.java.net/jdk/pull/6527

3 11

RFR: JDK-8278014: [vectorapi] Remove test run script
by Paul Sandoz 01 Dec '21

01 Dec '21

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. ------------- Commit messages: - JDK-8278014: [vectorapi] Remove test run script Changes: https://git.openjdk.java.net/jdk/pull/6621/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6621&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8278014 Stats: 158 lines in 5 files changed: 0 ins; 156 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/6621.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6621/head:pull/6621 PR: https://git.openjdk.java.net/jdk/pull/6621

4 7

RFR: 8193682: Infinite loop in ZipOutputStream.close()
by Ravi Reddy 01 Dec '21

01 Dec '21

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 ------------- Commit messages: - 8193682: Infinite loop in ZipOutputStream.close() Changes: https://git.openjdk.java.net/jdk/pull/5522/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5522&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8193682 Stats: 92 lines in 2 files changed: 87 ins; 0 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/5522.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5522/head:pull/5522 PR: https://git.openjdk.java.net/jdk/pull/5522

5 36

RFR: 8217496: Matcher.group() can return null after usePattern
by Ian Graves 01 Dec '21

01 Dec '21

Specification update to clarify Matcher behavior to include a null return value. ------------- Commit messages: - 8217496 Matcher.group() can return null after usePattern Changes: https://git.openjdk.java.net/jdk/pull/5827/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5827&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8217496 Stats: 6 lines in 1 file changed: 4 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/5827.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5827/head:pull/5827 PR: https://git.openjdk.java.net/jdk/pull/5827

2 1

RFR: 8277986: Typo in javadoc of java.util.zip.ZipEntry#setTime
by Jaikiran Pai 01 Dec '21

01 Dec '21

Can I please get a review for this change which fixes a typo in the javadoc of `java.util.zip.ZipEntry.setTime()` method? ------------- Commit messages: - 8277986: Typo in javadoc of java.util.zip.ZipEntry#setTime Changes: https://git.openjdk.java.net/jdk/pull/6615/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6615&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8277986 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/6615.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6615/head:pull/6615 PR: https://git.openjdk.java.net/jdk/pull/6615

4 5

RFR: 8190748: java/text/Format/DateFormat/DateFormatTest.java and NonGregorianFormatTest fail intermittently
by Naoto Sato 30 Nov '21

30 Nov '21

Fixing tests that fail at DST->STD offset transition. Simply skipping the tests on that occasion. ------------- Commit messages: - 8190748: java/text/Format/DateFormat/DateFormatTest.java and NonGregorianFormatTest fail intermittently Changes: https://git.openjdk.java.net/jdk/pull/6598/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6598&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8190748 Stats: 21 lines in 2 files changed: 14 ins; 2 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/6598.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6598/head:pull/6598 PR: https://git.openjdk.java.net/jdk/pull/6598

4 7

RFR: 8277606: String(String) constructor could copy hashIsZero
by PROgrm_JARvis 30 Nov '21

30 Nov '21

This is a trivial fix to make `String(String)` constructor copy the value of `hashIsZero` field. ------------- Commit messages: - fix: copy `hashIsZero` in `String#String(String)` Changes: https://git.openjdk.java.net/jdk/pull/6511/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6511&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8277606 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/6511.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6511/head:pull/6511 PR: https://git.openjdk.java.net/jdk/pull/6511

3 4