October 2021 - core-libs-dev

RFR: 8250678: ModuleDescriptor.Version parsing treats empty segments inconsistently
by Masanori Yano 09 Nov '21

09 Nov '21

Could you please review the 8250678 bug fixes? The `parse` method of ModuleDescriptor.Version class behaves incorrectly when the input string contains consecutive delimiters. The `parse` method treats strings as three sections, but the parsing method differs between the method for the version sections and the ones for others. In version sections, the `parse` method takes a single character in a loop and determines whether it is a delimiter. In pre and build sections, the parse method takes two characters in a loop and determines whether the second character is the delimiter. Therefore, if the string contains a sequence of delimiters in pre or build section, the `parse` method treats character at the odd-numbered position as a token and the one at even-numbered as a delimiter. A string containing consecutive delimiters is an incorrect version string, but this behavior does not follow the API specification. https://download.java.net/java/early_access/jdk18/docs/api/java.base/java/l… Therefore, I propose to fix the parsing method of pre and build section in the same way as the version. ------------- Commit messages: - 8250678: ModuleDescriptor.Version parsing treats empty segments inconsistently Changes: https://git.openjdk.java.net/jdk/pull/5679/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5679&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8250678 Stats: 26 lines in 2 files changed: 11 ins; 14 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5679.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5679/head:pull/5679 PR: https://git.openjdk.java.net/jdk/pull/5679

4 17

RFR: JDK-8274686 : java.util.UUID#hashCode() should use Long.hashCode()
by PROgrm_JARvis 08 Nov '21

08 Nov '21

This is trivial fix of [JDK-8274686](https://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-82746… which replaces manually-computed `int`-based `long` hash-code. Because `Long#hashCode(long)` uses other hashing function than the one currently used here: https://github.com/openjdk/jdk/blob/75d6688df9845ecb8f370b4cd2d5a36f13d3cdc… the value of `hashCode()` will produce different result, however this does not seem to be a breaking change as `UUID#hashCode()` is not specified. --- Note: the comment to the bug states: > Moved to JDK for further discussions of the proposed enhancement. But as I there seemed to be no corresponding discussion in core-libs-dev and the change looks trivial, I considered that it is appropriate to open a PR which (if needed) may be used for discussion (especially, considering the fact that its comments get mirrored to the ML). ------------- Commit messages: - fix: use `Long#hashCode(long)` for `UUID#hashCode()` Changes: https://git.openjdk.java.net/jdk/pull/5811/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5811&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8274686 Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5811.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5811/head:pull/5811 PR: https://git.openjdk.java.net/jdk/pull/5811

6 10

RFR: 8274412: Add a method to Stream API to consume and close the stream without using try-with-resources
by Tagir F.Valeev 06 Nov '21

06 Nov '21

Currently, when the stream holds a resource, it's necessary to wrap it with try-with-resources. This undermines the compact and fluent style of stream API calls. For example, if we want to get the `List` of files inside the directory and timely close the underlying filehandle, we should use something like this: List<Path> paths; try (Stream<Path> stream = Files.list(Path.of("/etc"))) { paths = stream.toList(); } // use paths I suggest to add a new default method to Stream interface named `consumeAndClose`, which allows performing terminal stream operation and closing the stream at the same time. It may look like this: default <R> R consumeAndClose(Function<? super Stream<T>, ? extends R> function) { Objects.requireNonNull(function); try(this) { return function.apply(this); } } Now, it will be possible to get the list of the files in the fluent manner: List<Path> list = Files.list(Path.of("/etc")).consumeAndClose(Stream::toList); ------------- Commit messages: - Fix tests - 8274412: Add a method to Stream API to consume and close the stream without using try-with-resources Changes: https://git.openjdk.java.net/jdk/pull/5796/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5796&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8274412 Stats: 140 lines in 5 files changed: 135 ins; 0 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/5796.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5796/head:pull/5796 PR: https://git.openjdk.java.net/jdk/pull/5796

9 13

RFR: 8275509: ModuleDescriptor.hashCode isn't reproducible across builds
by Jaikiran Pai 05 Nov '21

05 Nov '21

Can I please get a review for this change which fixes the issue reported in https://bugs.openjdk.java.net/browse/JDK-8275509? The `ModuleDescriptor.hashCode()` method uses the hash code of its various components to compute the final hash code. While doing so it ends up calling hashCode() on (collection of) various `enum` types. Since the hashCode() of enum types is generated from a call to `java.lang.Object.hashCode()`, their value isn't guaranteed to stay the same across multiple JVM runs. The commit here proposes to use the ordinal of the enum as the hash code. As Alan notes in the mailing list discussion, any changes to the ordinal of the enum (either due to addition of new value, removal of a value or just reordering existing values, all of which I think will be rare in these specific enum types) isn't expected to produce the same hash code across those changed runtimes and that should be okay. The rest of the ModuleDescriptor.hashCode() has been reviewed too and apart from calls to the enum types hashCode(), rest of the calls in that implementation, either directly or indirectly end up as calls on `java.lang.String.hashCode()` and as such aren't expected to cause non-deterministic values. A new jtreg test has been added to reproduce this issue and verify the fix. ------------- Commit messages: - minor test comment fix - 8275509: ModuleDescriptor.hashCode isn't reproducible across builds Changes: https://git.openjdk.java.net/jdk/pull/6078/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6078&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8275509 Stats: 178 lines in 2 files changed: 174 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/6078.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6078/head:pull/6078 PR: https://git.openjdk.java.net/jdk/pull/6078

5 36

RFR: 8275650: Problemlist java/io/File/createTempFile/SpecialTempFile.java for Windows 11
by Ivan Šipka 04 Nov '21

04 Nov '21

cc @ctornqvi ------------- Commit messages: - 8274122: added to problem list Changes: https://git.openjdk.java.net/jdk/pull/6025/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6025&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8275650 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/6025.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6025/head:pull/6025 PR: https://git.openjdk.java.net/jdk/pull/6025

5 22

RFR: 8276096: Simplify Unsafe.{load|store}Fence fallbacks by delegating to fullFence
by Aleksey Shipilev 04 Nov '21

04 Nov '21

`Unsafe.{load|store}Fence` falls back to `unsafe.cpp` for `OrderAccess::{acquire|release}Fence()`. It seems too heavy-handed (useless?) to call to runtime for a single memory barrier. We can simplify the native `Unsafe` interface by falling back to `fullFence` when `{load|store}Fence` intrinsics are not available. This would be similar to what `Unsafe.{loadLoad|storeStore}Fences` do. This is the behavior of these intrinsics now, on x86_64, using benchmarks from JDK-8276054: Benchmark Mode Cnt Score Error Units # Default Single.acquire avgt 3 0.407 ± 0.060 ns/op Single.full avgt 3 4.693 ± 0.005 ns/op Single.loadLoad avgt 3 0.415 ± 0.095 ns/op Single.plain avgt 3 0.406 ± 0.002 ns/op Single.release avgt 3 0.408 ± 0.047 ns/op Single.storeStore avgt 3 0.408 ± 0.043 ns/op # -XX:DisableIntrinsic=_storeFence Single.acquire avgt 3 0.408 ± 0.016 ns/op Single.full avgt 3 4.694 ± 0.002 ns/op Single.loadLoad avgt 3 0.406 ± 0.002 ns/op Single.plain avgt 3 0.406 ± 0.001 ns/op Single.release avgt 3 4.694 ± 0.003 ns/op <--- upgraded to full Single.storeStore avgt 3 4.690 ± 0.005 ns/op <--- upgraded to full # -XX:DisableIntrinsic=_loadFence Single.acquire avgt 3 4.691 ± 0.001 ns/op <--- upgraded to full Single.full avgt 3 4.693 ± 0.009 ns/op Single.loadLoad avgt 3 4.693 ± 0.013 ns/op <--- upgraded to full Single.plain avgt 3 0.408 ± 0.072 ns/op Single.release avgt 3 0.415 ± 0.016 ns/op Single.storeStore avgt 3 0.416 ± 0.041 ns/op # -XX:DisableIntrinsic=_fullFence Single.acquire avgt 3 0.406 ± 0.014 ns/op Single.full avgt 3 15.836 ± 0.151 ns/op <--- calls runtime Single.loadLoad avgt 3 0.406 ± 0.001 ns/op Single.plain avgt 3 0.426 ± 0.361 ns/op Single.release avgt 3 0.407 ± 0.021 ns/op Single.storeStore avgt 3 0.410 ± 0.061 ns/op # -XX:DisableIntrinsic=_fullFence,_loadFence Single.acquire avgt 3 15.822 ± 0.282 ns/op <--- upgraded, calls runtime Single.full avgt 3 15.851 ± 0.127 ns/op <--- calls runtime Single.loadLoad avgt 3 15.829 ± 0.045 ns/op <--- upgraded, calls runtime Single.plain avgt 3 0.406 ± 0.001 ns/op Single.release avgt 3 0.414 ± 0.156 ns/op Single.storeStore avgt 3 0.422 ± 0.452 ns/op # -XX:DisableIntrinsic=_fullFence,_storeFence Single.acquire avgt 3 0.407 ± 0.016 ns/op Single.full avgt 3 15.347 ± 6.783 ns/op <--- calls runtime Single.loadLoad avgt 3 0.406 ± 0.001 ns/op Single.plain avgt 3 0.406 ± 0.002 ns/op Single.release avgt 3 15.828 ± 0.019 ns/op <--- upgraded, calls runtime Single.storeStore avgt 3 15.834 ± 0.045 ns/op <--- upgraded, calls runtime # -XX:DisableIntrinsic=_fullFence,_loadFence,_storeFence Single.acquire avgt 3 15.838 ± 0.030 ns/op <--- upgraded, calls runtime Single.full avgt 3 15.854 ± 0.277 ns/op <--- calls runtime Single.loadLoad avgt 3 15.826 ± 0.160 ns/op <--- upgraded, calls runtime Single.plain avgt 3 0.406 ± 0.003 ns/op Single.release avgt 3 15.838 ± 0.019 ns/op <--- upgraded, calls runtime Single.storeStore avgt 3 15.844 ± 0.104 ns/op <--- upgraded, calls runtime Additional testing: - [x] Linux x86_64 fastdebug `tier1` ------------- Commit messages: - Fix Changes: https://git.openjdk.java.net/jdk/pull/6149/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6149&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8276096 Stats: 22 lines in 3 files changed: 6 ins; 11 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/6149.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6149/head:pull/6149 PR: https://git.openjdk.java.net/jdk/pull/6149

4 11

Incorrect assumption on the class name in compile(InputSource input, String name) at XSLTC.java
by Cheng Jin 03 Nov '21

03 Nov '21

Hi there, The code in compile(InputSource input, String name) at https://github.com/openjdk/jdk/blob/master/src/java.xml/share/classes/com/s… seems incorrect as follows: public boolean compile(InputSource input, String name) { try { // Reset globals in case we're called by compile(ArrayList v); reset(); // The systemId may not be set, so we'll have to check the URL String systemId = null; if (input != null) { systemId = input.getSystemId(); } // Set the translet class name if not already set if (_className == null) { if (name != null) { setClassName(name); } else if (systemId != null && !systemId.equals("")) { setClassName(Util.baseName(systemId)); <------- incorrect here } // Ensure we have a non-empty class name at this point if (_className == null || _className.length() == 0) { setClassName("GregorSamsa"); // default translet name } } ... Specifically, the code above assumes that systemId is something like "xxx:yyy" in which case the class name (_className) is "die.verwandlung.yyy" ("die.verwandlung." is the default package name since Java11) However,Util.baseName(systemId) returns null when systemId is something like "xxx:" (empty after ":"), in which case the class name (_className) ends up with "die.verwandlung."(an invalid class name inserted in the constant pool of the generated bytecode). >From the perspective of robustness, the code above should be updated to handle the situation. e.g. else if (systemId != null && !systemId.equals("")) { String baseName = Util.baseName(systemId); if ((baseName != null) && (baseName.length() > 0)) { <------ setClassName(baseName); } which means it should check whether the returned base name from Util.baseName(systemId) is empty before setting the class name; otherwise, it should use the default class name ("GregorSamsa"). Let me know if any other concern on this fix. Thanks and Best Regards Cheng Jin

1 3

8276207: Properties.loadFromXML/storeToXML works incorrectly for supplementary characters
by Anirvan Sarkar 02 Nov '21

02 Nov '21

Hi, Properties.loadFromXML/storeToXML works incorrectly for supplementary characters after JDK-8042889[1] was integrated in JDK 9. Properties.storeToXML now generates incorrect character references for supplementary characters. This is similar to JDK-8145974[2] which was fixed in the java.xml module in JDK 9. Properties.loadFromXML now fails to parse character references for supplementary characters and throws InvalidPropertiesFormatException. Sample program which demonstrates these issues is in the JBS[3]. Proposed patch to fix this issue is attached. JDK Tier 1 tests are all green. If it looks fine then I will create a pull request based on this. [1] : https://bugs.openjdk.java.net/browse/JDK-8042889 [2] : https://bugs.openjdk.java.net/browse/JDK-8145974 [3] : https://bugs.openjdk.java.net/browse/JDK-8276207 -- Anirvan

2 3

RFR: 8275766: (tz) Update Timezone Data to 2021e
by Yoshiki Sato 02 Nov '21

02 Nov '21

Please review the integration of tzdata2021e (including tzdata2021d) to the JDK. The fix has passed all relevant JTREG regression tests and JCK tests. 8275754: (tz) Update Timezone Data to 2021d 8275849: TestZoneInfo310.java fails with tzdata2021e ------------- Commit messages: - 8275754: (tz) Update Timezone Data to 2021d Changes: https://git.openjdk.java.net/jdk/pull/6144/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6144&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8275766 Stats: 52 lines in 6 files changed: 29 ins; 6 del; 17 mod Patch: https://git.openjdk.java.net/jdk/pull/6144.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6144/head:pull/6144 PR: https://git.openjdk.java.net/jdk/pull/6144

5 6

RFR: 8275735: [linux] Remove deprecated Metrics api (kernel memory limit)
by Severin Gehwolf 02 Nov '21

02 Nov '21

Please review this change to remove some API which no longer works as expected as recent OCI runtimes start to drop support for `--kernel-memory` switch. See the bug for references. This part of the API is not present in hotspot code. Testing: Container tests (cgroup v1) on Linux x86_64 (all pass) ------------- Commit messages: - 8275735: [linux] Remove deprecated Metrics api (kernel memory limit) Changes: https://git.openjdk.java.net/jdk/pull/6156/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6156&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8275735 Stats: 104 lines in 6 files changed: 0 ins; 100 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/6156.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6156/head:pull/6156 PR: https://git.openjdk.java.net/jdk/pull/6156

3 4