October 2021 - core-libs-dev

RFR: 8275342: Change nested classes in java.prefs to static nested classes
by Andrey Turbanov 02 Dec '21

02 Dec '21

Non-static classes hold a link to their parent classes, which can be avoided. ------------- Commit messages: - [PATCH] Change nested classes in java.prefs to static nested classes Changes: https://git.openjdk.java.net/jdk/pull/5971/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5971&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8275342 Stats: 20 lines in 3 files changed: 1 ins; 11 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/5971.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5971/head:pull/5971 PR: https://git.openjdk.java.net/jdk/pull/5971

3 4

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: 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: 8258117: jar tool sets the time stamp of module-info.class entries to the current time
by Jaikiran Pai 25 Nov '21

25 Nov '21

The commit here is a potential fix for the issue noted in https://bugs.openjdk.java.net/browse/JDK-8258117. The change here repurposes an existing internal interface `ModuleInfoEntry` to keep track of the last modified timestamp of a `module-info.class` descriptor. This commit uses the timestamp of the `module-info.class` on the filesystem to set the time on the `JarEntry`. There are a couple of cases to consider here: 1. When creating a jar (using `--create`), we use the source `module-info.class` from the filesystem and then add extended info to it (attributes like packages, module version etc...). In such cases, this patch will use the lastmodified timestamp from the filesystem of `module-info.class` even though we might end up updating/extending/modifying (for example by adding a module version) its content while storing it as a `JarEntry`. 2. When updating a jar (using `--update`), this patch will use the lastmodified timestamp of `module-info.class` either from the filesystem or from the source jar's entry (depending on whether a new `module-info.class` is being passed to the command). Here too, it's possible that we might end up changing/modifying/extending the `module-info.class` (for example, changing the module version to a new version) that gets written into the updated jar file, but this patch _won't_ use `System.currentTimeMillis()` even in such cases. If we do have to track actual changes that might happen to `module-info.class` while extending its info (in `extendedInfoBytes()`) and then decide whether to use current system time as last modified time, then this will require a bigger change and also a discussion on what kind of extending of module-info.class content will require a change to the lastmodifiedtime of that entry. ------------- Commit messages: - 8258117: jar tool sets the time stamp of module-info.class entries to the current time Changes: https://git.openjdk.java.net/jdk/pull/5486/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5486&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8258117 Stats: 317 lines in 2 files changed: 298 ins; 0 del; 19 mod Patch: https://git.openjdk.java.net/jdk/pull/5486.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5486/head:pull/5486 PR: https://git.openjdk.java.net/jdk/pull/5486

7 29

RFR: 8275063: Implementation of Memory Access API (Second incubator)
by Maurizio Cimadamore 24 Nov '21

24 Nov '21

This PR contains the API and implementation changes for JEP-419 [1]. A more detailed description of such changes, to avoid repetitions during the review process, is included as a separate comment. [1] - https://openjdk.java.net/jeps/419 ------------- Commit messages: - Fix issues with Aarch64 VaList implementation - Drop argument type profiling for removed `MemoryAccess` class - Add missing files - Tweak javadoc for MemeorySegment/MemoryAddress::setUtf8String - Initial push from panama/foreign Changes: https://git.openjdk.java.net/jdk/pull/5907/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5907&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8275063 Stats: 14262 lines in 186 files changed: 6583 ins; 5144 del; 2535 mod Patch: https://git.openjdk.java.net/jdk/pull/5907.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5907/head:pull/5907 PR: https://git.openjdk.java.net/jdk/pull/5907

7 49

RFR: 8217501: Matcher.hitEnd returns false for incomplete surrogate pairs
by Ian Graves 23 Nov '21

23 Nov '21

Fixing a bug where character matcher doesn't mark hitEnd as true if it's a code point with more than one character. ------------- Commit messages: - 8217501: Matcher.hitEnd returns false for incomplete surrogate pairs Changes: https://git.openjdk.java.net/jdk/pull/5725/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5725&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8217501 Stats: 26 lines in 2 files changed: 26 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/5725.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5725/head:pull/5725 PR: https://git.openjdk.java.net/jdk/pull/5725

2 1

RFR: 8274544: Langtools command's usage were grabled on Japanese Windows
by Ichiroh Takiguchi 22 Nov '21

22 Nov '21

JEP-400 (UTF-8 by Default) was eabled on JDK18-b13. After JDK18-b13, javac and some other langtool command's usage were garbled on Japanese Windows. These commands use PrintWriter instead of standard out/err with PrintStream. ------------- Commit messages: - Langtools command's usage were grabled on Japanese Windows Changes: https://git.openjdk.java.net/jdk/pull/5771/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5771&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8274544 Stats: 35 lines in 2 files changed: 34 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5771.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5771/head:pull/5771 PR: https://git.openjdk.java.net/jdk/pull/5771

6 48

RFR: 8275386: Change nested classes in jdk.jlink to static nested classes
by Andrey Turbanov 19 Nov '21

19 Nov '21

Non-static classes hold a link to their parent classes, which can be avoided. ------------- Commit messages: - [PATCH] Change nested classes in jdk.jlink to static nested classes Changes: https://git.openjdk.java.net/jdk/pull/5983/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5983&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8275386 Stats: 6 lines in 3 files changed: 0 ins; 0 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/5983.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5983/head:pull/5983 PR: https://git.openjdk.java.net/jdk/pull/5983

4 4

RFR: 8274946: Cleanup unnecessary calls to Throwable.initCause() in java.rmi
by Andrey Turbanov 19 Nov '21

19 Nov '21

Pass cause exception as constructor parameter is shorter and easier to read. ------------- Commit messages: - [PATCH] Cleanup unnecessary calls to Throwable.initCause() in java.rmi module Changes: https://git.openjdk.java.net/jdk/pull/5854/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5854&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8274946 Stats: 20 lines in 5 files changed: 0 ins; 10 del; 10 mod Patch: https://git.openjdk.java.net/jdk/pull/5854.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5854/head:pull/5854 PR: https://git.openjdk.java.net/jdk/pull/5854

4 3