[jdk17] Integrated: 8268717: Upstream: 8268673: Stack walk across optimized entry frame on fresh native thread fails
On Wed, 16 Jun 2021 11:19:37 GMT, Jorn Vernee wrote: > Upstream a critical fix from the panama-foreign repo. > > See the prior review thread here: > https://github.com/openjdk/panama-foreign/pull/558 > > Testing: tier 1-2, local run of run-test-jdk_foreign. This pull request has now been integrated. Changeset: f25e7197 Author:Jorn Vernee URL: https://git.openjdk.java.net/jdk17/commit/f25e7197fef76cc87a15da7cc96a42b84d69bbfe Stats: 198 lines in 12 files changed: 197 ins; 0 del; 1 mod 8268717: Upstream: 8268673: Stack walk across optimized entry frame on fresh native thread fails Reviewed-by: mcimadamore, erikj - PR: https://git.openjdk.java.net/jdk17/pull/76
Re: RFR: 8153490: Cannot setBytes() if incoming buffer's length is bigger than number of elements we want to insert. [v4]
On Mon, 24 May 2021 11:18:57 GMT, Mitsuru Kariya wrote: >> Fix `SerialBlob.setBytes(long pos, byte[] bytes, int offset, int length)` in >> the following cases: >> >> 1. `pos - 1 + bytes.length - offset > this.length() && pos - 1 + length <= >> this.length()` >>The original implementation throws `ArrayIndexOutOfBoundsException` but >> this case should end successfully. >>(test31) >> >> 2. `pos - 1 + length > this.length()` >>The original implementation throws `ArrayIndexOutOfBoundsException` but >> this case should end successfully. *1 >>(test32) >> >> 3. `pos == this.length() + 1` >>The original implementation throws `SerialException` but this case should >> end successfully. *2 >>(test33) >> >> 4. `length < 0` >>The original implementation throws `ArrayIndexOutOfBoundsException` but >> this case should throw `SerialException`. >>(test34) >> >> 5. `offset + length > Integer.MAX_VALUE` >>The original implementation throws `ArrayIndexOutOfBoundsException` (or >> `OutOfMemoryError` in most cases) but this case should throw >> `SerialException`. >>(test35) >> >> Additionally, fix `SerialClob.setString(long pos, String str, int offset, >> int length)` in the following cases: >> >> 1. `offset > str.length()` >>The original implementaion throws `StringIndexOutOfBoundsException` but >> this case should throw `SerialException`. >>(test39) >> >> 2. `pos - 1 + str.length() - offset > this.length() && pos - 1 + length <= >> this.length()` >>The original implementation throws `ArrayIndexOutOfBoundsException` but >> this case should end successfully. >>(test32) >> >> 3. `pos - 1 + length > this.length()` >>The original implementaion throws `SerialException` but this case should >> end successfully. *3 >>(test40) >> >> 4. `pos == this.length() + 1` >>The original implementaion throws `SerialException` but this case should >> end successfully. *4 >>(test41) >> >> 5. `length < 0` >>The original implementation throws `StringIndexOutOfBoundsException` but >> this case should throw `SerialException`. >>(test42) >> >> 6. `offset + length > Integer.MAX_VALUE` >>The original implementation throws `ArrayIndexOutOfBoundsException` (or >> `OutOfMemoryError` in most cases) but this case should throw >> `SerialException`. >>(test43) >> >> >> The javadoc has also been modified according to the above. >> >> *1 The documentation of `Blob.setBytes()` says, "If the end of the Blob >> value is reached while writing the array of bytes, then the length of the >> Blob value will be increased to accommodate the extra bytes." >> >> *2 The documentation of `Blob.setBytes()` says, "If the value specified for >> pos is greater than the length+1 of the BLOB value then the behavior is >> undefined." >>So, it should work correctly when pos == length+1 of the BLOB value. >> >> *3 The documentation of `Clob.setString()` says, "If the end of the Clob >> value is eached while writing the given string, then the length of the Clob >> value will be increased to accommodate the extra characters." >> >> *4 The documentation of `Clob.setString()` says, "If the value specified for >> pos is greater than the length+1 of the CLOB value then the behavior is >> undefined." >>So, it should work correctly when pos == length+1 of the CLOB value. > > Mitsuru Kariya has updated the pull request incrementally with one additional > commit since the last revision: > > Modify javadoc for consistency I hope to get to the CSR this week. Sorry for the delay - PR: https://git.openjdk.java.net/jdk/pull/4001
Re: [jdk17] RFR: JDK-8266310: deadlock between System.loadLibrary and JNI FindClass loading another class
On Fri, 18 Jun 2021 09:50:49 GMT, Aleksei Voitylov wrote: > Resubmitting the following PR https://github.com/openjdk/jdk/pull/3976 > against JDK17. > > This fixes the deadlock in ClassLoader between the two lock objects - a lock > object associated with the class being loaded, and the > ClassLoader.loadedLibraryNames hash map, locked during the native library > load operation. > > Further details can be found in the original PR. > > Testing: jtreg and jck testing with no regressions. A new regression test was > developed. @voitylov JDK 17 is an RDP 1 and I wonder if this long standing issue is critical or not. I've skimmed through the changes and I don't see any issue but if this is really intended for openjdk/jdk17 then I think it would be useful to have more eyes on it. - PR: https://git.openjdk.java.net/jdk17/pull/96
Re: RFR: 8268974: jpackage fails to handle --dest option containing "bin" folder
On Fri, 18 Jun 2021 22:46:24 GMT, Alexey Semenyuk wrote: > GetApplicationHomeFromDll() fails if the path to libjli.so contains "bin" > component (/tmp/bin/HelloWorld/lib/runtime/lib/libjli.so). TruncatePath() > looks for "/bin/" first in "/tmp/bin/HelloWorld/lib/runtime/lib/libjli.so" > string and then it looks for "/lib/". But this is wrong order as it should > look for "/lib/" first. I.e. TruncatePath() should look for "/bin/" and then > for "/lib/" if called from GetApplicationHome() and for "/lib/" first and > then for "/bin/" if called from GetApplicationHomeFromDll(). The test should use java launcher dynamically linked to libjli.so. So the standard java launcher wouldn't work. I can provide C source code of a test java launcher dynamically linked to libjli.so though. The test will need to compile java launcher from the source code. This looks more sophisticated compared to providing another jpackage test for this use case. - PR: https://git.openjdk.java.net/jdk/pull/4534
Re: RFR: 8268974: jpackage fails to handle --dest option containing "bin" folder [v2]
> GetApplicationHomeFromDll() fails if the path to libjli.so contains "bin" > component (/tmp/bin/HelloWorld/lib/runtime/lib/libjli.so). TruncatePath() > looks for "/bin/" first in "/tmp/bin/HelloWorld/lib/runtime/lib/libjli.so" > string and then it looks for "/lib/". But this is wrong order as it should > look for "/lib/" first. I.e. TruncatePath() should look for "/bin/" and then > for "/lib/" if called from GetApplicationHome() and for "/lib/" first and > then for "/bin/" if called from GetApplicationHomeFromDll(). Alexey Semenyuk has updated the pull request incrementally with one additional commit since the last revision: - Comments updated. - pathisso -> pathisdll. - Changes: - all: https://git.openjdk.java.net/jdk/pull/4534/files - new: https://git.openjdk.java.net/jdk/pull/4534/files/7028e74f..18aa7427 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4534&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4534&range=00-01 Stats: 13 lines in 1 file changed: 8 ins; 1 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/4534.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4534/head:pull/4534 PR: https://git.openjdk.java.net/jdk/pull/4534
Re: [jdk17] RFR: JDK-8266310: deadlock between System.loadLibrary and JNI FindClass loading another class
On Fri, 18 Jun 2021 09:50:49 GMT, Aleksei Voitylov wrote: > Resubmitting the following PR https://github.com/openjdk/jdk/pull/3976 > against JDK17. > > This fixes the deadlock in ClassLoader between the two lock objects - a lock > object associated with the class being loaded, and the > ClassLoader.loadedLibraryNames hash map, locked during the native library > load operation. > > Further details can be found in the original PR. > > Testing: jtreg and jck testing with no regressions. A new regression test was > developed. @dholmes-ora @ChrisHegarty @plevart reviewed PR openjdk/jdk#3976. Can you re-review this PR? - PR: https://git.openjdk.java.net/jdk17/pull/96
Re: [jdk17] RFR: JDK-8268826: Cleanup Override in Context-Specific Deserialization Filters [v2]
> Remove the unnecessary special case "OVERRIDE" in jdk.serialFilterFactory > property. > Fix description in the example of a filter allowing platform classes. > Suppress some warnings about use of SecurityManager in tests. Roger Riggs has updated the pull request incrementally with one additional commit since the last revision: Remove jdk.serialFilterFactory "OVERRIDE" special handling. Correct description of the example of allowing platform or bootstrap classes in OIF. Cleanup of logging of filter tracing. - Changes: - all: https://git.openjdk.java.net/jdk17/pull/85/files - new: https://git.openjdk.java.net/jdk17/pull/85/files/9471e4e0..0add8a1c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk17&pr=85&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk17&pr=85&range=00-01 Stats: 61 lines in 6 files changed: 9 ins; 26 del; 26 mod Patch: https://git.openjdk.java.net/jdk17/pull/85.diff Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/85/head:pull/85 PR: https://git.openjdk.java.net/jdk17/pull/85
Re: [jdk17] RFR: JDK-8266310: deadlock between System.loadLibrary and JNI FindClass loading another class
On Mon, 21 Jun 2021 13:26:14 GMT, Alan Bateman wrote: >> Resubmitting the following PR https://github.com/openjdk/jdk/pull/3976 >> against JDK17. >> >> This fixes the deadlock in ClassLoader between the two lock objects - a lock >> object associated with the class being loaded, and the >> ClassLoader.loadedLibraryNames hash map, locked during the native library >> load operation. >> >> Further details can be found in the original PR. >> >> Testing: jtreg and jck testing with no regressions. A new regression test >> was developed. > > @voitylov JDK 17 is an RDP 1 and I wonder if this long standing issue is > critical or not. I've skimmed through the changes and I don't see any issue > but if this is really intended for openjdk/jdk17 then I think it would be > useful to have more eyes on it. @AlanBateman I think it's worth to be included in JDK17 because in case someone hits this (like netty), the impact is high (a framework or an application can't start) and there is no viable workaround without rewriting code. The scope of change in the last version, at the same time, is limited. - PR: https://git.openjdk.java.net/jdk17/pull/96
RFR: 8266901: Clarify the method description of Duration.toDaysPart()
Please review this doc clarification fix to `toDaysPart()` method. CSR will also be filed accordingly. - Commit messages: - 8266901: Clarify the method description of Duration.toDaysPart() Changes: https://git.openjdk.java.net/jdk/pull/4542/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4542&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8266901 Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4542.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4542/head:pull/4542 PR: https://git.openjdk.java.net/jdk/pull/4542
Re: RFR: 8266901: Clarify the method description of Duration.toDaysPart()
On Mon, 21 Jun 2021 18:22:26 GMT, Naoto Sato wrote: > Please review this doc clarification fix to `toDaysPart()` method. CSR will > also be filed accordingly. Looks fine. - Marked as reviewed by bpb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4542
Re: RFR: 8266901: Clarify the method description of Duration.toDaysPart()
On Mon, 21 Jun 2021 18:22:26 GMT, Naoto Sato wrote: > Please review this doc clarification fix to `toDaysPart()` method. CSR will > also be filed accordingly. Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4542
Re: RFR: 8266901: Clarify the method description of Duration.toDaysPart()
On Mon, 21 Jun 2021 18:22:26 GMT, Naoto Sato wrote: > Please review this doc clarification fix to `toDaysPart()` method. CSR will > also be filed accordingly. Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4542
Re: RFR: 8266901: Clarify the method description of Duration.toDaysPart()
On Mon, 21 Jun 2021 18:22:26 GMT, Naoto Sato wrote: > Please review this doc clarification fix to `toDaysPart()` method. CSR will > also be filed accordingly. Associated CSR also reviewed. - Marked as reviewed by iris (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4542
[jdk17] RFR: JDK-8268404: [TESTBUG] tools/jpackage/windows/WinInstallerIconTest.java failed "AssertionError: Failed: Check icon"
…t.java failed "AssertionError: Failed: Check icon" - Commit messages: - JDK-8268404: [TESTBUG] tools/jpackage/windows/WinInstallerIconTest.java failed "AssertionError: Failed: Check icon" Changes: https://git.openjdk.java.net/jdk17/pull/111/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk17&pr=111&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8268404 Stats: 79 lines in 2 files changed: 0 ins; 59 del; 20 mod Patch: https://git.openjdk.java.net/jdk17/pull/111.diff Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/111/head:pull/111 PR: https://git.openjdk.java.net/jdk17/pull/111
Re: [jdk17] RFR: JDK-8268404: [TESTBUG] tools/jpackage/windows/WinInstallerIconTest.java failed "AssertionError: Failed: Check icon"
On Mon, 21 Jun 2021 20:21:58 GMT, Andy Herrick wrote: > …t.java failed "AssertionError: Failed: Check icon" Changes requested by asemenyuk (Reviewer). test/jdk/tools/jpackage/windows/WinInstallerIconTest.java line 75: > 73: > 74: // Create another installer with custom icon. > 75: long size3 = createInstaller(customIcon, "WithCustom3Icon"); The test should produce WinInstallerIconTest-1.0.exe artifact referenced in SQE test spec. But this patch will not produce it. It will produce WinInstallerIconTestWithDefaultIcon-1.0.exe, WinInstallerIconTestWithCustom2Icon-1.0.exe and WinInstallerIconTestWithCustom3Icon-1.0.exe artifacts and all of them will be deleted. test/jdk/tools/jpackage/windows/WinInstallerIconTest.java line 114: > 112: size = installerExePath[0].toFile().length(); > 113: TKit.trace(" installer: " + installerExePath[0] + " - size: > " + size); > 114: if (nameSuffix != "3") { Value of `nameSuffix` parameter is never equals to "3" - PR: https://git.openjdk.java.net/jdk17/pull/111
Re: RFR: 8268698: Use Objects.check{Index, FromToIndex, FromIndexSize} where possible [v6]
On Mon, 21 Jun 2021 05:17:09 GMT, Yi Yang wrote: >> After JDK-8265518(#3615), it's possible to replace all variants of >> checkIndex by >> Objects.checkIndex/Objects.checkFromToIndex/Objects.checkFromIndexSize in >> the whole JDK codebase. > > Yi Yang has updated the pull request incrementally with one additional commit > since the last revision: > > more replacement 2 All the updates to the check* methods look ok (requires some careful looking!). I cannot recall what others said about the change in exception messages. @jddarcy any advice here? src/java.base/share/classes/jdk/internal/util/Preconditions.java line 78: > 76: = Preconditions.outOfBoundsExceptionFormatter(new > StringIndexOutOfBoundsExceptionProducer()); > 77: > 78: public static final BiFunction, > StringIndexOutOfBoundsException> AIOOBE_FORMATTER Using incorrect exception type. Suggest you embed as inner class rather than separate declaration, since they are only used in one place. - PR: https://git.openjdk.java.net/jdk/pull/4507
Re: [jdk17] RFR: 8268888: Upstream 8268230: Foreign Linker API & Windows user32/kernel32: String conversion seems broken [v2]
On Wed, 16 Jun 2021 22:23:45 GMT, Jorn Vernee wrote: >> Upstream fix for 8268230 to mainline JDK. >> >> Prior review thread can be found here: >> https://github.com/openjdk/panama-foreign/pull/554 >> >> Testing: jdk_foreign test suite on Windows and Linux (WSL). > > Jorn Vernee has updated the pull request incrementally with one additional > commit since the last revision: > > Exclude test on unsupported platforms > > This test indirectly initializes CABI, which fails on Linux x86 Looks good - Marked as reviewed by mcimadamore (Reviewer). PR: https://git.openjdk.java.net/jdk17/pull/77
Re: RFR: 8266901: Clarify the method description of Duration.toDaysPart()
On Mon, 21 Jun 2021 18:22:26 GMT, Naoto Sato wrote: > Please review this doc clarification fix to `toDaysPart()` method. CSR will > also be filed accordingly. Marked as reviewed by scolebourne (Author). - PR: https://git.openjdk.java.net/jdk/pull/4542
RFR: Merge jdk17
Forwardport JDK 17 -> JDK 18 - Commit messages: - Merge - 8268672: C2: assert(!loop->is_member(u_loop)) failed: can be in outer loop or out of both loops only - 8268717: Upstream: 8268673: Stack walk across optimized entry frame on fresh native thread fails - 8268362: [REDO] C2 crash when compile negative Arrays.copyOf length after loop - 8268702: JFR diagnostic commands lack argument descriptors when viewed using Platform MBean Server - 8267042: bug in monitor locking/unlocking on ARM32 C1 due to uninitialized BasicObjectLock::_displaced_header - 8269063: Build failure due to VerifyReceiverTypes was not declared after JDK-8268405 The webrevs contain the adjustments done while merging with regards to each parent branch: - master: https://webrevs.openjdk.java.net/?repo=jdk&pr=4545&range=00.0 - jdk17: https://webrevs.openjdk.java.net/?repo=jdk&pr=4545&range=00.1 Changes: https://git.openjdk.java.net/jdk/pull/4545/files Stats: 608 lines in 25 files changed: 578 ins; 17 del; 13 mod Patch: https://git.openjdk.java.net/jdk/pull/4545.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4545/head:pull/4545 PR: https://git.openjdk.java.net/jdk/pull/4545
[jdk17] RFR: 8269096: Add java.util.Objects.newIdentity method
Add java.util.Objects.newIdentity to supply a unique object with identity. This is a replacement code can be used today for the traditional new Object() idiom, which will be deprecated under Project Valhalla. Refer to [JEP 401: Primitive Objects (Preview)](https://openjdk.java.net/jeps/401) for background. - Commit messages: - Copyright update and cleanup punctuation - 8269096: Add java.util.Objects.newIdentity method Changes: https://git.openjdk.java.net/jdk17/pull/112/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk17&pr=112&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8269096 Stats: 29 lines in 2 files changed: 28 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk17/pull/112.diff Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/112/head:pull/112 PR: https://git.openjdk.java.net/jdk17/pull/112
Re: [jdk17] RFR: JDK-8268826: Cleanup Override in Context-Specific Deserialization Filters [v2]
On Mon, 21 Jun 2021 17:15:03 GMT, Roger Riggs wrote: >> Remove the unnecessary special case "OVERRIDE" in jdk.serialFilterFactory >> property. >> Fix description in the example of a filter allowing platform classes. >> Suppress some warnings about use of SecurityManager in tests. > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Remove jdk.serialFilterFactory "OVERRIDE" special handling. > Correct description of the example of allowing platform or bootstrap > classes in OIF. > Cleanup of logging of filter tracing. Marked as reviewed by bchristi (Reviewer). - PR: https://git.openjdk.java.net/jdk17/pull/85
Re: [jdk17] RFR: 8269096: Add java.util.Objects.newIdentity method
On Mon, 21 Jun 2021 22:25:49 GMT, Roger Riggs wrote: > Add java.util.Objects.newIdentity to supply a unique object with identity. > This is a replacement code can be used today for the traditional new Object() > idiom, which will be deprecated under Project Valhalla. > Refer to [JEP 401: Primitive Objects > (Preview)](https://openjdk.java.net/jeps/401) for background. Looks okay in general. It'd be useful for the javadoc of `Object()` constructor to suggest using `Objects::newIdentity` instead of the constructor. - PR: https://git.openjdk.java.net/jdk17/pull/112
Re: [jdk17] RFR: JDK-8268404: [TESTBUG] tools/jpackage/windows/WinInstallerIconTest.java failed "AssertionError: Failed: Check icon"
On Mon, 21 Jun 2021 20:42:54 GMT, Alexey Semenyuk wrote: >> …t.java failed "AssertionError: Failed: Check icon" > > test/jdk/tools/jpackage/windows/WinInstallerIconTest.java line 75: > >> 73: >> 74: // Create another installer with custom icon. >> 75: long size3 = createInstaller(customIcon, "WithCustom3Icon"); > > The test should produce WinInstallerIconTest-1.0.exe artifact referenced in > SQE test spec. But this patch will not produce it. It will produce > WinInstallerIconTestWithDefaultIcon-1.0.exe, > WinInstallerIconTestWithCustom2Icon-1.0.exe and > WinInstallerIconTestWithCustom3Icon-1.0.exe artifacts and all of them will be > deleted. yes - I was changing the names to be the same length in attempt to create repeatable exe size - but that doesn't matter, so will restore the suffix's to what they were so WinInstallerIconTest-1.0.exe will be left for JPKG001-024 in Test Spec. - PR: https://git.openjdk.java.net/jdk17/pull/111
Integrated: Merge jdk17
On Mon, 21 Jun 2021 22:03:59 GMT, Jesper Wilhelmsson wrote: > Forwardport JDK 17 -> JDK 18 This pull request has now been integrated. Changeset: 0458113c Author:Jesper Wilhelmsson URL: https://git.openjdk.java.net/jdk/commit/0458113c6b1cf500ffdf049c1e3a698b16ce12ce Stats: 608 lines in 25 files changed: 578 ins; 17 del; 13 mod Merge - PR: https://git.openjdk.java.net/jdk/pull/4545
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 52 commits: - Merge - 8268458: Add verification type for evacuation failures Reviewed-by: kbarrett, iwalulya - 8268952: Automatically update heap sizes in G1MonitoringScope Reviewed-by: kbarrett, iwalulya - 8269029: compiler/codegen/TestCharVect2.java fails for client VMs Reviewed-by: kvn, jiefu - 8017163: G1: Refactor remembered sets 8048504: G1: Investigate replacing the coarse and fine grained data structures in the remembered sets 6949259: G1: Merge sparse and fine remembered set hash tables Co-authored-by: Ivan Walulya Co-authored-by: Thomas Schatzl Reviewed-by: sjohanss, iwalulya - 8266082: AssertionError in Annotate.fromAnnotations with -Xdoclint Reviewed-by: vromero - Merge - 8267042: bug in monitor locking/unlocking on ARM32 C1 due to uninitialized BasicObjectLock::_displaced_header Co-authored-by: Chris Cole Reviewed-by: dsamersoff - 8268964: Remove unused ReferenceProcessorAtomicMutator Reviewed-by: tschatzl, pliden - 8268900: com/sun/net/httpserver/Headers.java: Fix indentation and whitespace Reviewed-by: dfuchs, chegar, michaelm - ... and 42 more: https://git.openjdk.java.net/jdk/compare/d3ad8cd3...40c550db - Changes: https://git.openjdk.java.net/jdk/pull/4545/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4545&range=01 Stats: 14796 lines in 230 files changed: 12511 ins; 1719 del; 566 mod Patch: https://git.openjdk.java.net/jdk/pull/4545.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4545/head:pull/4545 PR: https://git.openjdk.java.net/jdk/pull/4545
Re: [jdk17] RFR: JDK-8266310: deadlock between System.loadLibrary and JNI FindClass loading another class
On Fri, 18 Jun 2021 09:50:49 GMT, Aleksei Voitylov wrote: > Resubmitting the following PR https://github.com/openjdk/jdk/pull/3976 > against JDK17. > > This fixes the deadlock in ClassLoader between the two lock objects - a lock > object associated with the class being loaded, and the > ClassLoader.loadedLibraryNames hash map, locked during the native library > load operation. > > Further details can be found in the original PR. > > Testing: jtreg and jck testing with no regressions. A new regression test was > developed. I've re-reviewed the locking aspect of this, but can't comment on the NativeLibraryContext changes. Thanks, David - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk17/pull/96
Re: [jdk17] RFR: 8267421: j.l.constant.DirectMethodHandleDesc.Kind.valueOf(int) implementation doesn't conform to the spec regarding REF_invokeInterface handling [v3]
> This PR is a copy of [PR-4416](https://github.com/openjdk/jdk/pull/4416) > which was intended to openjdk/jdk. > > Please review this PR which is syncing the implementation of > `DirectMethodHandleDesc.Kind.valueOf(int)` and > `DirectMethodHandleDesc.Kind.valueOf(int, boolean)` with its spec. My reading > of the method's spec is that if the value of the refKind parameter is: > MethodHandleInfo.REF_invokeInterface, then > DirectMethodHandleDesc.Kind.valueOf(int, boolean) should be called with a > value of true for its second argument, currently it is invoked with false > regardless of the value of the refKind parameter > > TIA Vicente Romero 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 three additional commits since the last revision: - Merge branch 'master' into JDK-8267421 - updating after review comments - 8267421: j.l.constant.DirectMethodHandleDesc.Kind.valueOf(int) implementation doesn't conform to the spec regarding REF_invokeInterface handling - Changes: - all: https://git.openjdk.java.net/jdk17/pull/29/files - new: https://git.openjdk.java.net/jdk17/pull/29/files/8ed4cdb3..be1a932c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk17&pr=29&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk17&pr=29&range=01-02 Stats: 16865 lines in 275 files changed: 10155 ins; 6060 del; 650 mod Patch: https://git.openjdk.java.net/jdk17/pull/29.diff Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/29/head:pull/29 PR: https://git.openjdk.java.net/jdk17/pull/29
Re: [jdk17] RFR: 8267421: j.l.constant.DirectMethodHandleDesc.Kind.valueOf(int) implementation doesn't conform to the spec regarding REF_invokeInterface handling [v2]
On Fri, 11 Jun 2021 18:17:10 GMT, Vicente Romero wrote: >> This PR is a copy of [PR-4416](https://github.com/openjdk/jdk/pull/4416) >> which was intended to openjdk/jdk. >> >> Please review this PR which is syncing the implementation of >> `DirectMethodHandleDesc.Kind.valueOf(int)` and >> `DirectMethodHandleDesc.Kind.valueOf(int, boolean)` with its spec. My >> reading of the method's spec is that if the value of the refKind parameter >> is: MethodHandleInfo.REF_invokeInterface, then >> DirectMethodHandleDesc.Kind.valueOf(int, boolean) should be called with a >> value of true for its second argument, currently it is invoked with false >> regardless of the value of the refKind parameter >> >> TIA > > Vicente Romero has updated the pull request incrementally with one additional > commit since the last revision: > > updating after review comments Marked as reviewed by mchung (Reviewer). src/java.base/share/classes/java/lang/constant/DirectMethodHandleDesc.java line 138: > 136: int i = tableIndex(refKind, isInterface); > 137: if (i >= 2 && i < TABLE.length) { > 138: return TABLE[i]; This fix looks good. The resulting `TABLE` has two entries per `refKind` for `isInterface` = `true` and `false`. test/jdk/java/lang/constant/MethodHandleDescTest.java line 379: > 377: for (int refKind : isInterfaceIgnored) { > 378: assertEquals(Kind.valueOf(refKind, false), > Kind.valueOf(refKind, true)); > 379: } Can you also add the test cases to verify `Kind::valueOf` with `REF_invokeStatic` and `REF_invokeSpecial`? - PR: https://git.openjdk.java.net/jdk17/pull/29
Re: [jdk17] RFR: 8267421: j.l.constant.DirectMethodHandleDesc.Kind.valueOf(int) implementation doesn't conform to the spec regarding REF_invokeInterface handling [v4]
> This PR is a copy of [PR-4416](https://github.com/openjdk/jdk/pull/4416) > which was intended to openjdk/jdk. > > Please review this PR which is syncing the implementation of > `DirectMethodHandleDesc.Kind.valueOf(int)` and > `DirectMethodHandleDesc.Kind.valueOf(int, boolean)` with its spec. My reading > of the method's spec is that if the value of the refKind parameter is: > MethodHandleInfo.REF_invokeInterface, then > DirectMethodHandleDesc.Kind.valueOf(int, boolean) should be called with a > value of true for its second argument, currently it is invoked with false > regardless of the value of the refKind parameter > > TIA Vicente Romero has updated the pull request incrementally with one additional commit since the last revision: addressing review comments - Changes: - all: https://git.openjdk.java.net/jdk17/pull/29/files - new: https://git.openjdk.java.net/jdk17/pull/29/files/be1a932c..d36a5528 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk17&pr=29&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk17&pr=29&range=02-03 Stats: 9 lines in 1 file changed: 8 ins; 1 del; 0 mod Patch: https://git.openjdk.java.net/jdk17/pull/29.diff Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/29/head:pull/29 PR: https://git.openjdk.java.net/jdk17/pull/29
Re: [jdk17] RFR: 8267421: j.l.constant.DirectMethodHandleDesc.Kind.valueOf(int) implementation doesn't conform to the spec regarding REF_invokeInterface handling [v2]
On Tue, 22 Jun 2021 00:45:36 GMT, Mandy Chung wrote: >> Vicente Romero has updated the pull request incrementally with one >> additional commit since the last revision: >> >> updating after review comments > > test/jdk/java/lang/constant/MethodHandleDescTest.java line 379: > >> 377: for (int refKind : isInterfaceIgnored) { >> 378: assertEquals(Kind.valueOf(refKind, false), >> Kind.valueOf(refKind, true)); >> 379: } > > Can you also add the test cases to verify `Kind::valueOf` with > `REF_invokeStatic` and `REF_invokeSpecial`? please see them below, thanks - PR: https://git.openjdk.java.net/jdk17/pull/29
[jdk17] Integrated: 8267421: j.l.constant.DirectMethodHandleDesc.Kind.valueOf(int) implementation doesn't conform to the spec regarding REF_invokeInterface handling
On Fri, 11 Jun 2021 15:01:20 GMT, Vicente Romero wrote: > This PR is a copy of [PR-4416](https://github.com/openjdk/jdk/pull/4416) > which was intended to openjdk/jdk. > > Please review this PR which is syncing the implementation of > `DirectMethodHandleDesc.Kind.valueOf(int)` and > `DirectMethodHandleDesc.Kind.valueOf(int, boolean)` with its spec. My reading > of the method's spec is that if the value of the refKind parameter is: > MethodHandleInfo.REF_invokeInterface, then > DirectMethodHandleDesc.Kind.valueOf(int, boolean) should be called with a > value of true for its second argument, currently it is invoked with false > regardless of the value of the refKind parameter > > TIA This pull request has now been integrated. Changeset: 6b14c8a1 Author:Vicente Romero URL: https://git.openjdk.java.net/jdk17/commit/6b14c8a1e5316b6c8584e93ee7a94d9eaec676cf Stats: 36 lines in 2 files changed: 23 ins; 8 del; 5 mod 8267421: j.l.constant.DirectMethodHandleDesc.Kind.valueOf(int) implementation doesn't conform to the spec regarding REF_invokeInterface handling Reviewed-by: mchung - PR: https://git.openjdk.java.net/jdk17/pull/29
Re: RFR: 8268698: Use Objects.check{Index, FromToIndex, FromIndexSize} where possible [v6]
On Mon, 21 Jun 2021 20:49:56 GMT, Paul Sandoz wrote: >> Yi Yang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> more replacement 2 > > src/java.base/share/classes/jdk/internal/util/Preconditions.java line 78: > >> 76: = Preconditions.outOfBoundsExceptionFormatter(new >> StringIndexOutOfBoundsExceptionProducer()); >> 77: >> 78: public static final BiFunction, >> StringIndexOutOfBoundsException> AIOOBE_FORMATTER > > Using incorrect exception type. Suggest you embed as inner class rather than > separate declaration, since they are only used in one place. Fixed. FYI: Current exception message looks like this: Exception in thread "main" java.lang.StringIndexOutOfBoundsException: Range [3, 1) out of bounds for length 6 at CheckIndex$StringIndexOutOfBoundsExceptionProducer.apply(CheckIndex.java:77) at CheckIndex$StringIndexOutOfBoundsExceptionProducer.apply(CheckIndex.java:72) at java.base/jdk.internal.util.Preconditions$1.apply(Preconditions.java:159) at java.base/jdk.internal.util.Preconditions$1.apply(Preconditions.java:156) at java.base/jdk.internal.util.Preconditions.outOfBounds(Preconditions.java:62) at java.base/jdk.internal.util.Preconditions.outOfBoundsCheckFromToIndex(Preconditions.java:76) at java.base/jdk.internal.util.Preconditions.checkFromToIndex(Preconditions.java:295) at CheckIndex.main(CheckIndex.java:110) I think now it expresses more exception information than before(and more consistent). - PR: https://git.openjdk.java.net/jdk/pull/4507
Re: RFR: 8268698: Use Objects.check{Index, FromToIndex, FromIndexSize} where possible [v7]
> After JDK-8265518(#3615), it's possible to replace all variants of checkIndex > by Objects.checkIndex/Objects.checkFromToIndex/Objects.checkFromIndexSize in > the whole JDK codebase. Yi Yang has updated the pull request incrementally with one additional commit since the last revision: correct exception type; use anonymous inner classes - Changes: - all: https://git.openjdk.java.net/jdk/pull/4507/files - new: https://git.openjdk.java.net/jdk/pull/4507/files/3a8875ec..2330ee38 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4507&range=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4507&range=05-06 Stats: 21 lines in 1 file changed: 0 ins; 9 del; 12 mod Patch: https://git.openjdk.java.net/jdk/pull/4507.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4507/head:pull/4507 PR: https://git.openjdk.java.net/jdk/pull/4507
Re: RFR: 8268698: Use Objects.check{Index, FromToIndex, FromIndexSize} where possible [v7]
On Tue, 22 Jun 2021 02:39:01 GMT, Yi Yang wrote: >> After JDK-8265518(#3615), it's possible to replace all variants of >> checkIndex by >> Objects.checkIndex/Objects.checkFromToIndex/Objects.checkFromIndexSize in >> the whole JDK codebase. > > Yi Yang has updated the pull request incrementally with one additional commit > since the last revision: > > correct exception type; use anonymous inner classes I found that after solving the problem that Preconditions cannot be used during the VM startup, a series of functions such as String.checkIndex/checkOffset/.. can also be harmlessly replaced, but this changeset is somewhat large and may overwrite the previous review progress. If it will confuse the current review progress for reviewers involving in this PR, I'd like to file a new PR to do this change later. - PR: https://git.openjdk.java.net/jdk/pull/4507
RFR: 8268664: The documentation of the Scanner.hasNextLine is incorrect
8268664: The documentation of the Scanner.hasNextLine is incorrect - Commit messages: - Clarifying docs on Scanner.hasNextLine() regarding line separators Changes: https://git.openjdk.java.net/jdk/pull/4547/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4547&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8268664 Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4547.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4547/head:pull/4547 PR: https://git.openjdk.java.net/jdk/pull/4547