February 2021 - core-libs-dev

RFR: 8261179: Norwegian Bokmål Locale fallback issue
by Naoto Sato 05 Feb '21

05 Feb '21

Please review this fix. The bug was revealed while porting CLDR v39 into the JDK, where they switch the Norwegian Bokmal from "nb" to "no". [1] Some regression tests started failing with it because COMPAT provider's "nb" resource was used according to the incorrect candidate locale order. [1] https://unicode-org.atlassian.net/browse/CLDR-2698 ------------- Commit messages: - 8261179: Norwegian Bokmål Locale fallback issue Changes: https://git.openjdk.java.net/jdk/pull/2414/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2414&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8261179 Stats: 43 lines in 2 files changed: 22 ins; 1 del; 20 mod Patch: https://git.openjdk.java.net/jdk/pull/2414.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2414/head:pull/2414 PR: https://git.openjdk.java.net/jdk/pull/2414

2 2

Re: RFR: 8254702: jpackage app launcher crashes on CentOS [v6]
by Alexey Semenyuk 05 Feb '21

05 Feb '21

> Fix for https://bugs.openjdk.java.net/browse/JDK-8254702 > > The fix splits Linux app launcher in app launcher and launcher shared lib. App launcher is pure C and doesn't have C++ code. App launcher lib incorporates bulk of C++ code from app launcher. > At startup app launcher loads launcher shared lib and calls functions it provides to get data for launching JVM (path to jli lib and arguments for JLI_Launch function call). > App launcher unloads launcher shared lib before launching JVM to remove C++ runtime from the process memory. > > Getting rid of C++ code from app launcher required to rewrite app installation location lookup code from C++ to C. LinuxPackage.c source is C alternative for https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/ap… and https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/ap…. > > Layout of jpackage's native code changed: > - `common`: code shared between all jpackage binaries. > - `applauncher`: launcher only code. > - `applauncherlib`: launcher lib code on Linux and launcher code on other platforms. > - `applaunchercommon`: code shared between launcher and launcher lib on Linux and launcher code on other platforms. Alexey Semenyuk has updated the pull request incrementally with one additional commit since the last revision: 8254702: jpackage app launcher crashes on CentOS ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/2320/files - new: https://git.openjdk.java.net/jdk/pull/2320/files/b2413570..3e73e074 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2320&range=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2320&range=04-05 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/2320.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2320/head:pull/2320 PR: https://git.openjdk.java.net/jdk/pull/2320

3 2

Re: RFR: 8254702: jpackage app launcher crashes on CentOS [v2]
by Alexey Semenyuk 05 Feb '21

05 Feb '21

> Fix for https://bugs.openjdk.java.net/browse/JDK-8254702 > > The fix splits Linux app launcher in app launcher and launcher shared lib. App launcher is pure C and doesn't have C++ code. App launcher lib incorporates bulk of C++ code from app launcher. > At startup app launcher loads launcher shared lib and calls functions it provides to get data for launching JVM (path to jli lib and arguments for JLI_Launch function call). > App launcher unloads launcher shared lib before launching JVM to remove C++ runtime from the process memory. > > Getting rid of C++ code from app launcher required to rewrite app installation location lookup code from C++ to C. LinuxPackage.c source is C alternative for https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/ap… and https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/ap…. > > Layout of jpackage's native code changed: > - `common`: code shared between all jpackage binaries. > - `applauncher`: launcher only code. > - `applauncherlib`: launcher lib code on Linux and launcher code on other platforms. > - `applaunchercommon`: code shared between launcher and launcher lib on Linux and launcher code on other platforms. Alexey Semenyuk has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: 8254702: jpackage app launcher crashes on CentOS ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/2320/files - new: https://git.openjdk.java.net/jdk/pull/2320/files/14d277a2..b493bcfd Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2320&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2320&range=00-01 Stats: 0 lines in 0 files changed: 0 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/2320.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2320/head:pull/2320 PR: https://git.openjdk.java.net/jdk/pull/2320

5 8

Re: RFR: 8195744: Avoid calling ClassLoader.checkPackageAccess if security manager is not installed [v3]
by Coleen Phillimore 05 Feb '21

05 Feb '21

> This change does not call up to Java for checkPackageAccess if the security manager is NULL, but still saves the protection domain in the pd_set for that dictionary entry. If the option -Djava.security.manager=disallow is set, that means that there will never be a security manager and the JVM code can avoid saving the protection domains completely. > See the two functions java_lang_System::has_security_manager() and java_lang_System::allow_security_manager() for details. > Also deleted ProtectionDomainVerification because there's no use for this option. > > Tested with tier1 hotspot, jdk and langtools. > and tier2-6. Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision: Fix comments and copyright. ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/2410/files - new: https://git.openjdk.java.net/jdk/pull/2410/files/296d0adb..7a2a3617 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2410&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2410&range=01-02 Stats: 14 lines in 2 files changed: 3 ins; 3 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/2410.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2410/head:pull/2410 PR: https://git.openjdk.java.net/jdk/pull/2410

2 2

RFR: 8180352: Add Stream.toList() method
by Donald Raab 05 Feb '21

05 Feb '21

I have been reading previous threads, the original bug request, and exploring the javadoc and implementation of toList() on Stream in JDK 16. I don’t want to waste time rehashing previous discussions, but I want to understand and prioritize the motivation of this change, and propose what I believe is a safer alternative name for this method based on the current implementation: Stream.toUnmodifiableList(). The original justification for Stream.toList() as described in the bug request filed in 2017 was: "It makes the coding just easier, saves some time and place.” see: https://bugs.openjdk.java.net/browse/JDK-8180352 Summarizing my interpretation of the eventual motivations for the change, over various threads (please correct as needed): 1. Convenience (easier to discover API and less code for developers to write - potential refactoring of large number of unmodifiable usages of Collectors.toList()) 2. Performance (less copying during parallel operations and pre-sizing for serial operations) 3. Unmodifiability (List result but thread-safe and usable in static contexts and as hash keys - unsafe for mutating operations) 4. Support for nulls (Inconsistent with Collectors.toUnmodifiableList() but consistent with other Stream methods) List is a mutable interface. It’s contract allows for “conditional mutability”. A convention was established in 2014 with Collectors.toList() returning a mutable List (ArrayList). The spec in Collectors.toList() says there is no guarantee on the type, but there appears to be no appetite to undo this. The current Collectors.toList() convention was then further strengthened with the addition of Collectors.toUnmodifiableList() in Java 10 in 2018. The problem as I see it is that we are currently only looking inside of the JDK for convention. The convention of using the toList() name has existed in other libraries and application code for as long as List has been available as an interface. The problem of inconsistency of convention within the JDK itself can be argued away as a historical mistake, but harder to claim for the "established convention" in other libraries. We learned an important lesson in JDK 15 when the addition of a default isEmpty method to CharSequence broke Eclipse Collections. See this excellent blog from Stuart Marks: https://stuartmarks.wordpress.com/2020/09/22/incompatibilities-with-jdk-15-…. The Stream interface has existed for seven years now. The method toList() is a prime candidate for anyone who wanted to optimize for convenience over the past seven years. I found this default definition of toList() on the Folds interface in Cyclops: https://github.com/aol/cyclops/blob/dc26bdad0f2b8a5aa87de4aa946ed52a0dbf00d… I discovered these occurrences of toList() in the jOOQ library. https://github.com/jOOQ/jOOL/blob/main/jOOL/src/main/java/org/jooq/lambda/C… https://github.com/jOOQ/jOOL/blob/main/jOOL/src/main/java/org/jooq/lambda/S… Incidentally, this interface also defines toUnmodifiableList(). https://github.com/jOOQ/jOOL/blob/main/jOOL/src/main/java/org/jooq/lambda/C… jOOQ did not define it’s toList() method as a default method on Collectable, so it should not trigger the default method issue we discovered in JDK 15, but the specifications of toList() on Collectable and Stream are incompatible. Which specification should win? Eclipse Collections consistent convention in all of its interfaces is that toList returns a MutableList [1]. MutableList extends java.util.List, which is a mutable interface. The Stream.toList() method will violate the principle of least surprise for Eclipse Collections users based on long time established conventions for the the library. There may be other libraries and applications that have defined their own toList() methods to return List as well. Lastly, Craig Motlin explains the performance optimization we use for toList in our parallel implementation in his talk in 2014: https://www.infoq.com/presentations/java-streams-scala-parallel-collections/ Thanks, Don [1] Example usages of Eclipse Collections toList: // toList result is mutable for all of these usages with Eclipse Collections List list1 = mutableSet.toList(); List list2 = mutableSet.asLazy().toList(); List list3 = mutableSet.asParallel(Executors.newWorkStealingPool(), 10).toList(); List list4 = mutableSet.stream().collect(Collectors.toList()); List list5 = mutableSet.stream().collect(Collectors2.toList()); // toList result is currently unmodifiable in Stream List list6 = mutableSet.stream().toList();

3 2

Re: RFR: 8254702: jpackage app launcher crashes on CentOS [v5]
by Alexey Semenyuk 05 Feb '21

05 Feb '21

> Fix for https://bugs.openjdk.java.net/browse/JDK-8254702 > > The fix splits Linux app launcher in app launcher and launcher shared lib. App launcher is pure C and doesn't have C++ code. App launcher lib incorporates bulk of C++ code from app launcher. > At startup app launcher loads launcher shared lib and calls functions it provides to get data for launching JVM (path to jli lib and arguments for JLI_Launch function call). > App launcher unloads launcher shared lib before launching JVM to remove C++ runtime from the process memory. > > Getting rid of C++ code from app launcher required to rewrite app installation location lookup code from C++ to C. LinuxPackage.c source is C alternative for https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/ap… and https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/ap…. > > Layout of jpackage's native code changed: > - `common`: code shared between all jpackage binaries. > - `applauncher`: launcher only code. > - `applauncherlib`: launcher lib code on Linux and launcher code on other platforms. > - `applaunchercommon`: code shared between launcher and launcher lib on Linux and launcher code on other platforms. Alexey Semenyuk has updated the pull request incrementally with one additional commit since the last revision: 8254702: jpackage app launcher crashes on CentOS ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/2320/files - new: https://git.openjdk.java.net/jdk/pull/2320/files/b6c62a2a..b2413570 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2320&range=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2320&range=03-04 Stats: 2 lines in 1 file changed: 0 ins; 2 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/2320.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2320/head:pull/2320 PR: https://git.openjdk.java.net/jdk/pull/2320

1 0

Re: RFR: 8254702: jpackage app launcher crashes on CentOS [v4]
by Alexey Semenyuk 05 Feb '21

05 Feb '21

> Fix for https://bugs.openjdk.java.net/browse/JDK-8254702 > > The fix splits Linux app launcher in app launcher and launcher shared lib. App launcher is pure C and doesn't have C++ code. App launcher lib incorporates bulk of C++ code from app launcher. > At startup app launcher loads launcher shared lib and calls functions it provides to get data for launching JVM (path to jli lib and arguments for JLI_Launch function call). > App launcher unloads launcher shared lib before launching JVM to remove C++ runtime from the process memory. > > Getting rid of C++ code from app launcher required to rewrite app installation location lookup code from C++ to C. LinuxPackage.c source is C alternative for https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/ap… and https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/ap…. > > Layout of jpackage's native code changed: > - `common`: code shared between all jpackage binaries. > - `applauncher`: launcher only code. > - `applauncherlib`: launcher lib code on Linux and launcher code on other platforms. > - `applaunchercommon`: code shared between launcher and launcher lib on Linux and launcher code on other platforms. Alexey Semenyuk has updated the pull request incrementally with two additional commits since the last revision: - 8254702: jpackage app launcher crashes on CentOS - 8254702: jpackage app launcher crashes on CentOS ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/2320/files - new: https://git.openjdk.java.net/jdk/pull/2320/files/91744255..b6c62a2a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2320&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2320&range=02-03 Stats: 49 lines in 10 files changed: 26 ins; 11 del; 12 mod Patch: https://git.openjdk.java.net/jdk/pull/2320.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2320/head:pull/2320 PR: https://git.openjdk.java.net/jdk/pull/2320

1 0

RFR: 8261154: Memory leak in Java_java_lang_ClassLoader_defineClass0 with long class names
by Claes Redestad 05 Feb '21

05 Feb '21

This patch resolves a potential memory leak in Java_java_lang_ClassLoader_defineClass0 I've not figured a good way to write a regression test, but manually verified the leak and the fix by adding a microbenchmark that triggers the leak. When run with `-XX:NativeMemoryTracking=summary -XX:+UnlockDiagnosticVMOptions -XX:+PrintNMTStatistics` there will be a section of memory of unknown origin, e.g.: - Unknown (reserved=65536KB, committed=55104KB) (mmap: reserved=65536KB, committed=55104KB) With the patch, there is no memory unaccounted for. ------------- Commit messages: - Add code used to generate the X_LONG_NAME_BYTECODE - Add micros showcasing leak - Resolve memory leak in defineClass0 Changes: https://git.openjdk.java.net/jdk/pull/2407/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2407&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8261154 Stats: 88 lines in 2 files changed: 84 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/2407.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2407/head:pull/2407 PR: https://git.openjdk.java.net/jdk/pull/2407

4 10

Re: RFR: 8231436: Fix the applicability of a no-@Target annotation type
by Liam Miller-Cushon 05 Feb '21

05 Feb '21

On Thu, 4 Feb 2021 21:04:51 GMT, Liam Miller-Cushon <cushon(a)openjdk.org> wrote: >> Thanks, I was missing context here. I now realize this patch is fixing a bug related to [JDK-8254023](https://bugs.openjdk.java.net/browse/JDK-8254023)--when the handling of annotations without `@Target` was updated to allow them to be applied to module declarations, the handling of repeatable annotations without `@Target` should also have been updated. I filed a new bug for the sub-issue that this patch is fixing, and will update it accordingly in a minute: https://bugs.openjdk.java.net/browse/JDK-8261088 >> >> I'm also happy to help with any work to be done on the larger question of interpreting `@Target`-less annotations as applicable to type contexts, if there's agreement about what to do there. > > I spun the original change that related specifically to repeatable annotations and `@Target(MODULE)` out into https://github.com/openjdk/jdk/pull/2412 and filed corresponding CSR [JDK-8261181](https://bugs.openjdk.java.net/browse/JDK-8261181). > > I will look at updating this review to actually implement the change to make no-`@Target` annotations applicable to all contexts next. I pushed an initial implementation of a fix to make no-`@Target` annotation applicable in all contexts, including type contexts. I have updated affected tier 1 langtools tests, which provides some signal about the compatibility impact. That also uncovered a couple of bugs: one issue where javadoc isn't escaping type annotations in html ([JDK-8261203](https://bugs.openjdk.java.net/browse/JDK-8261203)) and a crash with intersection types and LVTI ([JDK-8261205](https://bugs.openjdk.java.net/browse/JDK-8261205)) ------------- PR: https://git.openjdk.java.net/jdk/pull/2303

1 0

Re: RFR: 8231436: Fix the applicability of a no-@Target annotation type [v2]
by Liam Miller-Cushon 05 Feb '21

05 Feb '21

> Please review this fix to add `ElementType.MODULE` to the default list of annotation targets, to allow annotations without an explicit `@Target` to be used on module declarations. Liam Miller-Cushon has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: 8231436: Fix the applicability of a no-@Target annotation type ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/2303/files - new: https://git.openjdk.java.net/jdk/pull/2303/files/b0d6abbc..7e1c404d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2303&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2303&range=00-01 Stats: 191 lines in 36 files changed: 115 ins; 36 del; 40 mod Patch: https://git.openjdk.java.net/jdk/pull/2303.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2303/head:pull/2303 PR: https://git.openjdk.java.net/jdk/pull/2303

1 0