Integrated: 8271208: Typo in ModuleDescriptor.read javadoc
On Thu, 5 Aug 2021 14:28:30 GMT, Jaikiran Pai wrote: > Can I please get a review of this trivial fix which fixes the typo noted in > https://bugs.openjdk.java.net/browse/JDK-8271208? > > Ran `make docs-image` locally and generated the new javadocs and the change > looks fine. This pull request has now been integrated. Changeset: e38e365c Author:Jaikiran Pai URL: https://git.openjdk.java.net/jdk/commit/e38e365c70197f7e45d8bdc7d6c2e3c59717369e Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod 8271208: Typo in ModuleDescriptor.read javadoc Reviewed-by: alanb, iris - PR: https://git.openjdk.java.net/jdk/pull/5021
Re: RFR: 8271208: Typo in ModuleDescriptor.read javadoc
On Thu, 5 Aug 2021 14:28:30 GMT, Jaikiran Pai wrote: > Can I please get a review of this trivial fix which fixes the typo noted in > https://bugs.openjdk.java.net/browse/JDK-8271208? > > Ran `make docs-image` locally and generated the new javadocs and the change > looks fine. Thank you for the reviews, Alan and Iris. - PR: https://git.openjdk.java.net/jdk/pull/5021
Re: RFR: Merge jdk17 [v2]
> Forwardport JDK 17 -> JDK 18 Jesper Wilhelmsson has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 440 commits: - Merge - 8271293: Monitor class should use ThreadBlockInVMPreprocess Reviewed-by: dholmes, dcubed - 8270116: Expand ButtonGroupLayoutTraversalTest.java to run in all LaFs, including Aqua on macOS Reviewed-by: psadhukhan, aivanov - 8271905: mark hotspot runtime/Metaspace tests which ignore external VM flags Reviewed-by: stuefe - 8271308: (fc) FileChannel.transferTo() transfers no more than Integer.MAX_VALUE bytes in one call Reviewed-by: alanb, vtewari - 8271953: fix mis-merge in JDK-8271878 Reviewed-by: jwilhelm, ctornqvi - 8267840: Improve URLStreamHandler.parseURL() Reviewed-by: dfuchs, redestad - 8271840: Add simple Integer.toString microbenchmarks Reviewed-by: shade - 8271121: ZGC: stack overflow (segv) when -Xlog:gc+start=debug Reviewed-by: ayang, eosterlund - 8270903: sun.net.httpserver.HttpConnection: Improve toString Reviewed-by: chegar, vtewari - ... and 430 more: https://git.openjdk.java.net/jdk/compare/dfacda48...7e069880 - Changes: https://git.openjdk.java.net/jdk/pull/5026/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5026=01 Stats: 97370 lines in 1522 files changed: 61891 ins; 27834 del; 7645 mod Patch: https://git.openjdk.java.net/jdk/pull/5026.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5026/head:pull/5026 PR: https://git.openjdk.java.net/jdk/pull/5026
Integrated: Merge jdk17
On Thu, 5 Aug 2021 23:49:48 GMT, Jesper Wilhelmsson wrote: > Forwardport JDK 17 -> JDK 18 This pull request has now been integrated. Changeset: 14692d5e Author:Jesper Wilhelmsson URL: https://git.openjdk.java.net/jdk/commit/14692d5ed0652b867fcf28baafa498a9441683ac Stats: 307 lines in 32 files changed: 129 ins; 33 del; 145 mod Merge - PR: https://git.openjdk.java.net/jdk/pull/5026
RFR: 8271820: Implementation of JEP 416: Reimplement Core Reflection with Method Handle
This reimplements core reflection with method handles. For `Constructor::newInstance` and `Method::invoke`, the new implementation uses `MethodHandle`. For `Field` accessor, the new implementation uses `VarHandle`.For the first few invocations of one of these reflective methods on a specific reflective object we invoke the corresponding method handle directly. After that we spin a dynamic bytecode stub defined in a hidden class which loads the target `MethodHandle` or `VarHandle` from its class data as a dynamically computed constant. Loading the method handle from a constant allows JIT to inline the method-handle invocation in order to achieve good performance. The VM's native reflection methods are needed during early startup, before the method-handle mechanism is initialized. That happens soon after System::initPhase1 and before System::initPhase2, after which we switch to using method handles exclusively. The core reflection and method handle implementation are updated to handle chained caller-sensitive method calls [1] properly. A caller-sensitive method can define with a caller-sensitive adapter method that will take an additional caller class parameter and the adapter method will be annotated with `@CallerSensitiveAdapter` for better auditing. See the detailed description from [2]. Ran tier1-tier8 tests. [1] https://bugs.openjdk.java.net/browse/JDK-8013527 [2] https://bugs.openjdk.java.net/browse/JDK-8271820?focusedCommentId=14439430=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14439430 - Commit messages: - fix whitespace - Merge branch 'master' of https://github.com/openjdk/jdk into reimplement-method-invoke - Merge branch 'master' of https://github.com/openjdk/jdk into reimplement-method-invoke - Merge - clean up test - Merge branch 'master' of https://github.com/openjdk/jdk into reimplement-method-invoke - cleanup - Merge branch 'master' of https://github.com/openjdk/jdk into reimplement-method-invoke - fix ClassByteBuilder to handle short field type properly. Support volatile fields - minor cleanup - ... and 5 more: https://git.openjdk.java.net/jdk/compare/cb368802...c575c3db Changes: https://git.openjdk.java.net/jdk/pull/5027/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5027=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8271820 Stats: 7572 lines in 75 files changed: 7103 ins; 288 del; 181 mod Patch: https://git.openjdk.java.net/jdk/pull/5027.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5027/head:pull/5027 PR: https://git.openjdk.java.net/jdk/pull/5027
RFR: Merge jdk17
Forwardport JDK 17 -> JDK 18 - Commit messages: - Merge - 8270872: Final nroff manpage update for JDK 17 - 8271588: JFR Recorder Thread crashed with SIGSEGV in write_klass - 8271863: ProblemList serviceability/sa/TestJmapCore.java on linux-x64 with ZGC The webrevs contain the adjustments done while merging with regards to each parent branch: - master: https://webrevs.openjdk.java.net/?repo=jdk=5026=00.0 - jdk17: https://webrevs.openjdk.java.net/?repo=jdk=5026=00.1 Changes: https://git.openjdk.java.net/jdk/pull/5026/files Stats: 307 lines in 32 files changed: 129 ins; 33 del; 145 mod Patch: https://git.openjdk.java.net/jdk/pull/5026.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5026/head:pull/5026 PR: https://git.openjdk.java.net/jdk/pull/5026
Re: RFR: 8271868: Warn user when using mac-sign option with unsigned app-image. [v2]
On Thu, 5 Aug 2021 17:07:13 GMT, Andy Herrick wrote: >> 8271868: Warn user when using mac-sign option with unsigned app-image. > > Andy Herrick has updated the pull request incrementally with one additional > commit since the last revision: > > JDK-8271868: Warn user when using mac-sign option with unsigned app-image. Marked as reviewed by almatvee (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5004
[jdk17] Integrated: JDK-8270872: Final nroff manpage update for JDK 17
On Thu, 5 Aug 2021 19:20:50 GMT, Jonathan Gibbons wrote: > Please review a semi-automatic update of the nroff man pages from the > upstream files. This pull request has now been integrated. Changeset: dfacda48 Author:Jonathan Gibbons URL: https://git.openjdk.java.net/jdk17/commit/dfacda488bfbe2e11e8d607a6d08527710286982 Stats: 289 lines in 27 files changed: 117 ins; 31 del; 141 mod 8270872: Final nroff manpage update for JDK 17 Reviewed-by: darcy, mr, iris, naoto - PR: https://git.openjdk.java.net/jdk17/pull/303
Re: [jdk17] RFR: JDK-8270872: Final nroff manpage update for JDK 17
On Thu, 5 Aug 2021 21:40:40 GMT, Naoto Sato wrote: >> According to the comments in the makefile >> (`closed/make/UpdateOpenManPages.gmk`) the copyright line is taken from the >> original Markdown file, so if the year is wrong there, it will be wrong in >> the generated nroff file. >> >> I think it would be incorrect to edit the dates locally in these files, >> because they'll just be overwritten when we generate the files again. >> Ideally, the dates should be fixed (if necessary) in the Markdown files, but >> that seems out of scope for this P1. >> >> This is "just" an issue with copyright dates in source files ... and yes, >> while I know copyright dates are important, this problem is arguably part of >> an ongoing more general problem. >> >> I note that the generated files *do* correctly identify themselves with >> `2021` in the visible output generated to the console by the `man` command. > > Thanks for the explanation, Jon. Fine by me. I agree that fixing this date is not necessary at this time. - PR: https://git.openjdk.java.net/jdk17/pull/303
Re: [jdk17] RFR: JDK-8270872: Final nroff manpage update for JDK 17
On Thu, 5 Aug 2021 21:36:58 GMT, Jonathan Gibbons wrote: >> src/jdk.hotspot.agent/share/man/jhsdb.1 line 1: >> >>> 1: .\" Copyright (c) 2019, 2020, Oracle and/or its affiliates. All rights >>> reserved. >> >> This seems not correct? > > According to the comments in the makefile > (`closed/make/UpdateOpenManPages.gmk`) the copyright line is taken from the > original Markdown file, so if the year is wrong there, it will be wrong in > the generated nroff file. > > I think it would be incorrect to edit the dates locally in these files, > because they'll just be overwritten when we generate the files again. > Ideally, the dates should be fixed (if necessary) in the Markdown files, but > that seems out of scope for this P1. > > This is "just" an issue with copyright dates in source files ... and yes, > while I know copyright dates are important, this problem is arguably part of > an ongoing more general problem. > > I note that the generated files *do* correctly identify themselves with > `2021` in the visible output generated to the console by the `man` command. Thanks for the explanation, Jon. Fine by me. - PR: https://git.openjdk.java.net/jdk17/pull/303
Re: [jdk17] RFR: JDK-8270872: Final nroff manpage update for JDK 17
On Thu, 5 Aug 2021 19:20:50 GMT, Jonathan Gibbons wrote: > Please review a semi-automatic update of the nroff man pages from the > upstream files. Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.java.net/jdk17/pull/303
Re: [jdk17] RFR: JDK-8270872: Final nroff manpage update for JDK 17
On Thu, 5 Aug 2021 19:57:59 GMT, Naoto Sato wrote: >> Please review a semi-automatic update of the nroff man pages from the >> upstream files. > > src/jdk.hotspot.agent/share/man/jhsdb.1 line 1: > >> 1: .\" Copyright (c) 2019, 2020, Oracle and/or its affiliates. All rights >> reserved. > > This seems not correct? According to the comments in the makefile (`closed/make/UpdateOpenManPages.gmk`) the copyright line is taken from the original Markdown file, so if the year is wrong there, it will be wrong in the generated nroff file. I think it would be incorrect to edit the dates locally in these files, because they'll just be overwritten when we generate the files again. Ideally, the dates should be fixed (if necessary) in the Markdown files, but that seems out of scope for this P1. This is "just" an issue with copyright dates in source files ... and yes, while I know copyright dates are important, this problem is arguably part of an ongoing more general problem. I note that the generated files *do* correctly identify themselves with `2021` in the visible output generated to the console by the `man` command. - PR: https://git.openjdk.java.net/jdk17/pull/303
Re: [jdk17] RFR: JDK-8270872: Final nroff manpage update for JDK 17
On Thu, 5 Aug 2021 19:20:50 GMT, Jonathan Gibbons wrote: > Please review a semi-automatic update of the nroff man pages from the > upstream files. Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.java.net/jdk17/pull/303
Re: [jdk17] RFR: JDK-8270872: Final nroff manpage update for JDK 17
On Thu, 5 Aug 2021 19:20:50 GMT, Jonathan Gibbons wrote: > Please review a semi-automatic update of the nroff man pages from the > upstream files. Marked as reviewed by mr (Lead). - PR: https://git.openjdk.java.net/jdk17/pull/303
Re: [jdk17] RFR: JDK-8270872: Final nroff manpage update for JDK 17
On Thu, 5 Aug 2021 19:20:50 GMT, Jonathan Gibbons wrote: > Please review a semi-automatic update of the nroff man pages from the > upstream files. src/jdk.hotspot.agent/share/man/jhsdb.1 line 1: > 1: .\" Copyright (c) 2019, 2020, Oracle and/or its affiliates. All rights > reserved. This seems not correct? - PR: https://git.openjdk.java.net/jdk17/pull/303
Re: [jdk17] RFR: JDK-8270872: Final nroff manpage update for JDK 17
On Thu, 5 Aug 2021 19:20:50 GMT, Jonathan Gibbons wrote: > Please review a semi-automatic update of the nroff man pages from the > upstream files. Marked as reviewed by darcy (Reviewer). - PR: https://git.openjdk.java.net/jdk17/pull/303
[jdk17] RFR: JDK-8270872: Final nroff manpage update for JDK 17
Please review a semi-automatic update of the nroff man pages from the upstream files. - Commit messages: - JDK-8270872: Final nroff manpage update for JDK 17 Changes: https://git.openjdk.java.net/jdk17/pull/303/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk17=303=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8270872 Stats: 289 lines in 27 files changed: 117 ins; 31 del; 141 mod Patch: https://git.openjdk.java.net/jdk17/pull/303.diff Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/303/head:pull/303 PR: https://git.openjdk.java.net/jdk17/pull/303
Re: RFR: 8271225: Add floorDivExact() method to java.lang.[Strict]Math [v3]
On Thu, 5 Aug 2021 18:57:42 GMT, Brian Burkhalter wrote: >> Add methods `floorDivExact(int,int)` and `floorDivExact(long,long)` to >> `java.lang.Math` and `java.lang.StrictMath`. > > Brian Burkhalter has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains five commits: > > - 8271225: Verbiage update after 8271599 > - Merge > - 8271225: Revert drive-by verbiage updates > - 8271225: Some verbiage updates > - 8271225: Add floorDivExact() method to java.lang.[Strict]Math CSR updated to match this PR. - PR: https://git.openjdk.java.net/jdk/pull/4941
Re: RFR: 8271868: Warn user when using mac-sign option with unsigned app-image. [v2]
On Thu, 5 Aug 2021 17:07:13 GMT, Andy Herrick wrote:
>> 8271868: Warn user when using mac-sign option with unsigned app-image.
>
> Andy Herrick has updated the pull request incrementally with one additional
> commit since the last revision:
>
> JDK-8271868: Warn user when using mac-sign option with unsigned app-image.
src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacBaseInstallerBundler.java
line 142:
> 140:
> SIGN_BUNDLE.fetchFrom(params)).orElse(Boolean.FALSE)) {
> 141: // if signing bundle with app-image, warn user if
> app-image
> 142: // is not allready signed.
nitpicking: typo "allready" -> "already"
-
PR: https://git.openjdk.java.net/jdk/pull/5004
Re: RFR: 8271225: Add floorDivExact() method to java.lang.[Strict]Math [v3]
> Add methods `floorDivExact(int,int)` and `floorDivExact(long,long)` to > `java.lang.Math` and `java.lang.StrictMath`. Brian Burkhalter has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains five commits: - 8271225: Verbiage update after 8271599 - Merge - 8271225: Revert drive-by verbiage updates - 8271225: Some verbiage updates - 8271225: Add floorDivExact() method to java.lang.[Strict]Math - Changes: https://git.openjdk.java.net/jdk/pull/4941/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4941=02 Stats: 205 lines in 3 files changed: 202 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/4941.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4941/head:pull/4941 PR: https://git.openjdk.java.net/jdk/pull/4941
Re: RFR: 8271868: Warn user when using mac-sign option with unsigned app-image. [v2]
On Thu, 5 Aug 2021 17:07:13 GMT, Andy Herrick wrote: >> 8271868: Warn user when using mac-sign option with unsigned app-image. > > Andy Herrick has updated the pull request incrementally with one additional > commit since the last revision: > > JDK-8271868: Warn user when using mac-sign option with unsigned app-image. will fix this spelling error - PR: https://git.openjdk.java.net/jdk/pull/5004
Re: RFR: 8271868: Warn user when using mac-sign option with unsigned app-image. [v2]
On Thu, 5 Aug 2021 17:07:13 GMT, Andy Herrick wrote: >> 8271868: Warn user when using mac-sign option with unsigned app-image. > > Andy Herrick has updated the pull request incrementally with one additional > commit since the last revision: > > JDK-8271868: Warn user when using mac-sign option with unsigned app-image. Marked as reviewed by asemenyuk (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5004
New candidate JEP: 416: Reimplement Core Reflection with Method Handles
https://openjdk.java.net/jeps/416 Summary: Reimplement java.lang.reflect.Method, Constructor, and Field on top of java.lang.invoke method handles. Making method handles the underlying mechanism for reflection will reduce the maintenance and development cost of both the java.lang.reflect and java.lang.invoke APIs. - Mark
Re: RFR: 8271868: Warn user when using mac-sign option with unsigned app-image. [v2]
On Thu, 5 Aug 2021 17:07:13 GMT, Andy Herrick wrote: >> 8271868: Warn user when using mac-sign option with unsigned app-image. > > Andy Herrick has updated the pull request incrementally with one additional > commit since the last revision: > > JDK-8271868: Warn user when using mac-sign option with unsigned app-image. now recording in AppImageFile if signing used to create the app-image, and showing warning only if signing an app using an app-image that is not so recorded as signed. - PR: https://git.openjdk.java.net/jdk/pull/5004
Re: RFR: 8271868: Warn user when using mac-sign option with unsigned app-image. [v2]
> 8271868: Warn user when using mac-sign option with unsigned app-image. Andy Herrick has updated the pull request incrementally with one additional commit since the last revision: JDK-8271868: Warn user when using mac-sign option with unsigned app-image. - Changes: - all: https://git.openjdk.java.net/jdk/pull/5004/files - new: https://git.openjdk.java.net/jdk/pull/5004/files/153e75ea..afc0f197 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5004=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5004=00-01 Stats: 72 lines in 6 files changed: 30 ins; 27 del; 15 mod Patch: https://git.openjdk.java.net/jdk/pull/5004.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5004/head:pull/5004 PR: https://git.openjdk.java.net/jdk/pull/5004
Re: RFR: 8271208: Typo in ModuleDescriptor.read javadoc
On Thu, 5 Aug 2021 14:28:30 GMT, Jaikiran Pai wrote: > Can I please get a review of this trivial fix which fixes the typo noted in > https://bugs.openjdk.java.net/browse/JDK-8271208? > > Ran `make docs-image` locally and generated the new javadocs and the change > looks fine. Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5021
Re: RFR: JDK-8266490: Extend the OSContainer API to support the pids controller of cgroups [v6]
On Thu, 5 Aug 2021 11:19:17 GMT, Matthias Baesken wrote: >>> What do you think about accepting, when setting -1/unlimited, a high limit >>> number like 20.000+ as well (and and a comment that on some setups >>> unlimited means just "high number" but not unlimited? >> >> This seems most reasonable. I'd suggest to accept a limit of `> 2` or >> `Unlimited` in the test output. In case of it NOT being `Unlimited` for the >> `--pids-limit=-1` case, I'd also include the actual output in logs with a >> message that it got accepted as unlimited. >> >>> Another Idea I had was to start a little test java program that creates >>> e.g. 50.000 (or another high number) of threads. If this fails with >>> "unilimited" pids-limit set, we might have a setup like yours and then skip >>> the test (or accept a high number like I suggested). >> >> This seems overkill and prone to failures, IMHO. > >> > What do you think about accepting, when setting -1/unlimited, a high limit >> > number like 20.000+ as well (and and a comment that on some setups >> > unlimited means just "high number" but not unlimited? >> >> This seems most reasonable. I'd suggest to accept a limit of `> 2` or >> `Unlimited` in the test output. In case of it NOT being `Unlimited` for the >> `--pids-limit=-1` case, I'd also include the actual output in logs with a >> message that it got accepted as unlimited. >> > > Hi Severin, I adjusted the tests so that in case of Unlimited, an integer > value > 2 is accepted as well. > Hopefully this addresses the issues observed on your setup. > Best regards, Matthias @MBaesken Thanks. I'll test it and will report back. - PR: https://git.openjdk.java.net/jdk/pull/4518
Integrated: 8271840: Add simple Integer.toString microbenchmarks
On Wed, 4 Aug 2021 07:46:48 GMT, Claes Redestad wrote: > This adds Integer.toString microbenchmarks, roughly similar to the > Long.toString microbenchmarks (toStringSmall should use an equivalent data > set). This is useful for comparison purposes and completeness. > > Also tuned existing benchmarks to harmonize setup, reduce runtime without > significantly harming reliability of results, and remove the explicit > Threads.MAX from Longs This pull request has now been integrated. Changeset: 55bd52a1 Author:Claes Redestad URL: https://git.openjdk.java.net/jdk/commit/55bd52a14283033d66cd7bf1deadb31c040b09c7 Stats: 43 lines in 2 files changed: 33 ins; 3 del; 7 mod 8271840: Add simple Integer.toString microbenchmarks Reviewed-by: shade - PR: https://git.openjdk.java.net/jdk/pull/4988
Re: RFR: 8271840: Add simple Integer.toString microbenchmarks
On Wed, 4 Aug 2021 08:04:48 GMT, Aleksey Shipilev wrote: > Looks fine. Thanks! - PR: https://git.openjdk.java.net/jdk/pull/4988
Re: RFR: 8271208: Typo in ModuleDescriptor.read javadoc
On Thu, 5 Aug 2021 14:28:30 GMT, Jaikiran Pai wrote: > Can I please get a review of this trivial fix which fixes the typo noted in > https://bugs.openjdk.java.net/browse/JDK-8271208? > > Ran `make docs-image` locally and generated the new javadocs and the change > looks fine. Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5021
RFR: 8271208: Typo in ModuleDescriptor.read javadoc
Can I please get a review of this trivial fix which fixes the typo noted in https://bugs.openjdk.java.net/browse/JDK-8271208? Ran `make docs-image` locally and generated the new javadocs and the change looks fine. - Commit messages: - 8271208: Typo in ModuleDescriptor.read javadoc Changes: https://git.openjdk.java.net/jdk/pull/5021/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5021=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8271208 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/5021.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5021/head:pull/5021 PR: https://git.openjdk.java.net/jdk/pull/5021
Re: RFR: JDK-8271858: Handle to jimage file inherited into child processes (posix)
On Thu, 5 Aug 2021 13:13:27 GMT, Alan Bateman wrote: >> Probably others too, if we care about inheriting read handles to child. >> >> The background is that I am playing with a new test which checks that the VM >> has no open inheritable file descriptors. It is supposed to replace some >> specialized tests which are maintenance-intensive. I am still tuning the >> test, since at the moment it turns out too many false positives. >> >> Anyway, this is the very first descriptor the VM opens, therefore it >> triggered my test. I thought since there is a sibling fix for windows, a >> similar fix for posix would be symmetric. >> >> If you think this is not worth fixing, or we should fix all occurrences in >> one swoop, that is fine too. > > On Unix systems, the JDK has always relied on the Runtime.exec implementation > to close the file descriptors. On Windows the inheritance has to be disabled > in the parent. So if the gtest launcher is forking directly then we may have > a bit of whack-a-mole to change all the places that open file descriptors. My idea was born since we have jtreg tests that assert that certain VM internal fds, namely of log files, are not handed down to child processes. The motivation originally was Windows, since child processes would block that file from being moved. The test is done for both Unix and Windows, however. It is fragile and difficult to analyze when it fails. I wanted to throw away the Unix portion of that test and replace it with a simple gtest, either checking CLOEXEC for just that particular fd, or (my current approach) for all handles. But if what you are saying is how we do things - we don't bother with CLOEXEC, just rely on Runtime.exec() to close all fds, and accept the fact that "foreign" forks will cause us problems - then I could just throw the Unix portion of that test away without replacing it with anything. ATM the libs/module fd seems to be the only file descriptor tripping up my test though. Maybe it's not so bad. I am mainly afraid of situations where the gtestlauncher runs in some instrumented form, maybe with a virus scanner in its belly, with foreign code opening fds without our knowledge. I am still unsure about which direction to go. - PR: https://git.openjdk.java.net/jdk/pull/4991
Re: RFR: JDK-8271858: Handle to jimage file inherited into child processes (posix)
On Wed, 4 Aug 2021 17:04:38 GMT, Thomas Stuefe wrote:
>> src/java.base/unix/native/libjimage/osSupport_unix.cpp line 41:
>>
>>> 39: */
>>> 40: jint osSupport::openReadOnly(const char *path) {
>>> 41: return ::open(path, O_CLOEXEC);
>>
>> This is okay but I think it would be useful to know why this one place needs
>> O_CLOEXEC and not others.
>
> Probably others too, if we care about inheriting read handles to child.
>
> The background is that I am playing with a new test which checks that the VM
> has no open inheritable file descriptors. It is supposed to replace some
> specialized tests which are maintenance-intensive. I am still tuning the
> test, since at the moment it turns out too many false positives.
>
> Anyway, this is the very first descriptor the VM opens, therefore it
> triggered my test. I thought since there is a sibling fix for windows, a
> similar fix for posix would be symmetric.
>
> If you think this is not worth fixing, or we should fix all occurrences in
> one swoop, that is fine too.
On Unix systems, the JDK has always relied on the Runtime.exec implementation
to close the file descriptors. On Windows the inheritance has to be disabled in
the parent. So if the gtest launcher is forking directly then we may have a bit
of whack-a-mole to change all the places that open file descriptors.
-
PR: https://git.openjdk.java.net/jdk/pull/4991
Re: RFR: JDK-8271858: Handle to jimage file inherited into child processes (posix)
On Wed, 4 Aug 2021 13:35:59 GMT, Alan Bateman wrote:
>> We should not leak the handle to the jimage file (lib/modules) to childs.
>>
>> JDK-8194734 did solve this for windows. We should use FD_CLOEXEC on Posix as
>> well.
>>
>> Note that this only poses a problem when a child process is spawned off not
>> via `Runtime.exec` but via another route since the spawnhelper closes all
>> file descriptors.
>>
>> ---
>> test:
>>
>> manually verified that lib/modules now has the FD_CLOEXEC flag set.
>
> src/java.base/unix/native/libjimage/osSupport_unix.cpp line 41:
>
>> 39: */
>> 40: jint osSupport::openReadOnly(const char *path) {
>> 41: return ::open(path, O_CLOEXEC);
>
> This is okay but I think it would be useful to know why this one place needs
> O_CLOEXEC and not others.
Probably others too, if we care about inheriting read handles to child.
The background is that I am playing with a new test which checks that the VM
has no open inheritable file descriptors. It is supposed to replace some
specialized tests which are maintenance-intensive. I am still tuning the test,
since at the moment it turns out too many false positives.
Anyway, this is the very first descriptor the VM opens, therefore it triggered
my test. I thought since there is a sibling fix for windows, a similar fix for
posix would be symmetric.
If you think this is not worth fixing, or we should fix all occurrences in one
swoop, that is fine too.
-
PR: https://git.openjdk.java.net/jdk/pull/4991
RFR: JDK-8271858: Handle to jimage file inherited into child processes (posix)
We should not leak the handle to the jimage file (lib/modules) to childs. JDK-8194734 did solve this for windows. We should use FD_CLOEXEC on Posix as well. Note that this only poses a problem when a child process is spawned off not via `Runtime.exec` but via another route since the spawnhelper closes all file descriptors. --- test: manually verified that lib/modules now has the FD_CLOEXEC flag set. - Commit messages: - open image file with O_CLOEXEC Changes: https://git.openjdk.java.net/jdk/pull/4991/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4991=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8271858 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4991.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4991/head:pull/4991 PR: https://git.openjdk.java.net/jdk/pull/4991
Re: RFR: JDK-8271858: Handle to jimage file inherited into child processes (posix)
On Wed, 4 Aug 2021 12:22:24 GMT, Thomas Stuefe wrote:
> We should not leak the handle to the jimage file (lib/modules) to childs.
>
> JDK-8194734 did solve this for windows. We should use FD_CLOEXEC on Posix as
> well.
>
> Note that this only poses a problem when a child process is spawned off not
> via `Runtime.exec` but via another route since the spawnhelper closes all
> file descriptors.
>
> ---
> test:
>
> manually verified that lib/modules now has the FD_CLOEXEC flag set.
src/java.base/unix/native/libjimage/osSupport_unix.cpp line 41:
> 39: */
> 40: jint osSupport::openReadOnly(const char *path) {
> 41: return ::open(path, O_CLOEXEC);
This is okay but I think it would be useful to know why this one place needs
O_CLOEXEC and not others.
-
PR: https://git.openjdk.java.net/jdk/pull/4991
Re: RFR: JDK-8266490: Extend the OSContainer API to support the pids controller of cgroups [v8]
> Hello, please review this PR; it extend the OSContainer API in order to also
> support the pids controller of cgroups.
>
> I noticed that unlike the other controllers "cpu", "cpuset", "cpuacct",
> "memory" on some older Linux distros (SLES 12.1, RHEL 7.1) the pids
> controller might not be there (or not fully supported) so it was added as
> optional , see the coding
>
>
> if (!cg_infos[PIDS_IDX]._data_complete) {
> log_debug(os, container)("Optional cgroup v1 pids subsystem not found");
> // keep the other controller info, pids is optional
> }
Matthias Baesken has updated the pull request incrementally with one additional
commit since the last revision:
Adjust tests, unlimited pids value can lead on some setups to high number
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/4518/files
- new: https://git.openjdk.java.net/jdk/pull/4518/files/39b50089..0dd58e00
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk=4518=07
- incr: https://webrevs.openjdk.java.net/?repo=jdk=4518=06-07
Stats: 68 lines in 2 files changed: 65 ins; 0 del; 3 mod
Patch: https://git.openjdk.java.net/jdk/pull/4518.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/4518/head:pull/4518
PR: https://git.openjdk.java.net/jdk/pull/4518
Re: RFR: JDK-8266490: Extend the OSContainer API to support the pids controller of cgroups [v6]
On Mon, 2 Aug 2021 11:42:33 GMT, Severin Gehwolf wrote: > > What do you think about accepting, when setting -1/unlimited, a high limit > > number like 20.000+ as well (and and a comment that on some setups > > unlimited means just "high number" but not unlimited? > > This seems most reasonable. I'd suggest to accept a limit of `> 2` or > `Unlimited` in the test output. In case of it NOT being `Unlimited` for the > `--pids-limit=-1` case, I'd also include the actual output in logs with a > message that it got accepted as unlimited. > Hi Severin, I adjusted the tests so that in case of Unlimited, an integer value > 2 is accepted as well. Hopefully this addresses the issues observed on your setup. Best regards, Matthias - PR: https://git.openjdk.java.net/jdk/pull/4518
Re: RFR: JDK-8266490: Extend the OSContainer API to support the pids controller of cgroups [v7]
> Hello, please review this PR; it extend the OSContainer API in order to also
> support the pids controller of cgroups.
>
> I noticed that unlike the other controllers "cpu", "cpuset", "cpuacct",
> "memory" on some older Linux distros (SLES 12.1, RHEL 7.1) the pids
> controller might not be there (or not fully supported) so it was added as
> optional , see the coding
>
>
> if (!cg_infos[PIDS_IDX]._data_complete) {
> log_debug(os, container)("Optional cgroup v1 pids subsystem not found");
> // keep the other controller info, pids is optional
> }
Matthias Baesken 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 seven additional
commits since the last revision:
- Merge remote-tracking branch 'origin/master' into JDK-8266490
- Minor adjustments, handling of Unlimited
- Merge remote-tracking branch 'origin/master' into JDK-8266490
- Add hotspot tests
- test and small adjustments suggested by Severin
- Adjustments following Severins comments
- JDK-8266490
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/4518/files
- new: https://git.openjdk.java.net/jdk/pull/4518/files/857ab1db..39b50089
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk=4518=06
- incr: https://webrevs.openjdk.java.net/?repo=jdk=4518=05-06
Stats: 4915 lines in 198 files changed: 3503 ins; 507 del; 905 mod
Patch: https://git.openjdk.java.net/jdk/pull/4518.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/4518/head:pull/4518
PR: https://git.openjdk.java.net/jdk/pull/4518
