Re: RFR: JDK-8275703: System.loadLibrary fails on Big Sur for libraries hidden from filesystem
On Wed, 27 Oct 2021 03:31:00 GMT, Jaikiran Pai wrote: >> On, macOS 11.x, system libraries are loaded from dynamic linker cache. The >> libraries are no longer present on the filesystem. >> `NativeLibraries::loadLibrary` checks for the file existence before calling >> `JVM_LoadLibrary`. Such check no longer applies on Big Sur. This >> proposes that on macOS >= 11, it will skip the file existence check and >> attempt to load a library for each path from java.library.path and system >> library path. > > src/java.base/share/classes/jdk/internal/loader/NativeLibraries.java line 166: > >> 164: return null; >> 165: } >> 166: return file.getCanonicalPath(); > > I think a `!file.exists()` check would still be needed here to handle the > case when `loadLibraryOnlyIfPresent` is `false`, isn't it? Ignore this previous comment. I read your JBS comment just now which says: > The JDK side determines the path of the given library name and checks if it > exists before calling JVM_LoadLibrary. A potential fix for MacOS might be to > skip the file presence check and pass it to JVM. So, I think, the whole point of this change in this block is to skip the file existence check and return back a file path. - PR: https://git.openjdk.java.net/jdk/pull/6127
Re: RFR: JDK-8275703: System.loadLibrary fails on Big Sur for libraries hidden from filesystem
On Tue, 26 Oct 2021 22:51:29 GMT, Mandy Chung wrote: > On, macOS 11.x, system libraries are loaded from dynamic linker cache. The > libraries are no longer present on the filesystem. > `NativeLibraries::loadLibrary` checks for the file existence before calling > `JVM_LoadLibrary`. Such check no longer applies on Big Sur. This proposes > that on macOS >= 11, it will skip the file existence check and attempt to > load a library for each path from java.library.path and system library path. src/java.base/share/classes/jdk/internal/loader/NativeLibraries.java line 166: > 164: return null; > 165: } > 166: return file.getCanonicalPath(); I think a `!file.exists()` check would still be needed here to handle the case when `loadLibraryOnlyIfPresent` is `false`, isn't it? - PR: https://git.openjdk.java.net/jdk/pull/6127
Re: RFR: JDK-8275703: System.loadLibrary fails on Big Sur for libraries hidden from filesystem
On Tue, 26 Oct 2021 22:51:29 GMT, Mandy Chung wrote: > On, macOS 11.x, system libraries are loaded from dynamic linker cache. The > libraries are no longer present on the filesystem. > `NativeLibraries::loadLibrary` checks for the file existence before calling > `JVM_LoadLibrary`. Such check no longer applies on Big Sur. This proposes > that on macOS >= 11, it will skip the file existence check and attempt to > load a library for each path from java.library.path and system library path. src/java.base/macosx/classes/jdk/internal/loader/ClassLoaderHelper.java line 44: > 42: } catch (NumberFormatException e) {} > 43: } > 44: hasDynamicLoaderCache = major >= 11; Hello Mandy, I'm not too familiar with MacOS versioning schemes. However, in this specific logic, if the `os.version` value doesn't contain a dot character, then the `major` is initialized to `11`, which would then evaluate this `hasDynamicLoaderCache` to `true`. That would mean if the `os.version` is (for example) `10`, then `hasDynamicLoaderCache` will be incorrectly set to `true` here, isn't it? - PR: https://git.openjdk.java.net/jdk/pull/6127
Re: RFR: JDK-8275703: System.loadLibrary fails on Big Sur for libraries hidden from filesystem
On Tue, 26 Oct 2021 22:51:29 GMT, Mandy Chung wrote: > On, macOS 11.x, system libraries are loaded from dynamic linker cache. The > libraries are no longer present on the filesystem. > `NativeLibraries::loadLibrary` checks for the file existence before calling > `JVM_LoadLibrary`. Such check no longer applies on Big Sur. This proposes > that on macOS >= 11, it will skip the file existence check and attempt to > load a library for each path from java.library.path and system library path. Hi Mandy, Hotspot changes are fine. Thanks, David - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6127
RFR: JDK-8275703: System.loadLibrary fails on Big Sur for libraries hidden from filesystem
On, macOS 11.x, system libraries are loaded from dynamic linker cache. The libraries are no longer present on the filesystem. `NativeLibraries::loadLibrary` checks for the file existence before calling `JVM_LoadLibrary`. Such check no longer applies on Big Sur. This proposes that on macOS >= 11, it will skip the file existence check and attempt to load a library for each path from java.library.path and system library path. - Commit messages: - trim whitespaces - add copyright header - fix typo - JDK-8275703: System.loadLibrary fails on Big Sur for libraries hidden from filesystem Changes: https://git.openjdk.java.net/jdk/pull/6127/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6127=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8275703 Stats: 198 lines in 9 files changed: 170 ins; 2 del; 26 mod Patch: https://git.openjdk.java.net/jdk/pull/6127.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6127/head:pull/6127 PR: https://git.openjdk.java.net/jdk/pull/6127
Re: RFR: 8275650: Problemlist java/io/File/createTempFile/SpecialTempFile.java for Windows 11 [v2]
On Tue, 26 Oct 2021 20:59:45 GMT, Igor Ignatyev wrote: >> Ivan Šipka has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains two commits: >> >> - fixed OS identifier >> - 8274122: added to problem list > > And now you use an incorrect bug id in the problem list, it should be 8274122 Oops! Thanks @iignatev ! - PR: https://git.openjdk.java.net/jdk/pull/6025
Re: RFR: 8275650: Problemlist java/io/File/createTempFile/SpecialTempFile.java for Windows 11 [v2]
On Tue, 26 Oct 2021 19:48:47 GMT, Ivan Šipka wrote: >> cc @ctornqvi > > Ivan Šipka 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 two additional commits since > the last revision: > > - fixed OS identifier > - 8274122: added to problem list And now you use an incorrect bug id in the problem list, it should be 8274122 - PR: https://git.openjdk.java.net/jdk/pull/6025
Re: RFR: 8275650: Problemlist java/io/File/createTempFile/SpecialTempFile.java for Windows 11 [v2]
On Tue, 26 Oct 2021 19:48:47 GMT, Ivan Šipka wrote: >> cc @ctornqvi > > Ivan Šipka 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 two additional commits since > the last revision: > > - fixed OS identifier > - 8274122: added to problem list Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6025
Re: RFR: 8274750: java/io/File/GetXSpace.java failed: '/dev': 191488 != 190976
On Tue, 26 Oct 2021 20:24:28 GMT, Naoto Sato wrote: >> Please consider this proposed change to ignore comparing the total size of >> `/dev` as returned by `DF(1)` and `STAT(2)` on macOS. > > test/jdk/java/io/File/GetXSpace.java line 210: > >> 208: if (Platform.isOSX() && s.name().equals("/dev")) { >> 209: out.println("/dev:\n Skipping size comparison for /dev on >> macOS"); >> 210: return; > > Should it call `pass()`? I don't know. As it is skipped it is not exactly passed but ignored. - PR: https://git.openjdk.java.net/jdk/pull/6122
Re: RFR: 8274750: java/io/File/GetXSpace.java failed: '/dev': 191488 != 190976
On Tue, 26 Oct 2021 19:00:44 GMT, Brian Burkhalter wrote: > Please consider this proposed change to ignore comparing the total size of > `/dev` as returned by `DF(1)` and `STAT(2)` on macOS. test/jdk/java/io/File/GetXSpace.java line 210: > 208: if (Platform.isOSX() && s.name().equals("/dev")) { > 209: out.println("/dev:\n Skipping size comparison for /dev on > macOS"); > 210: return; Should it call `pass()`? - PR: https://git.openjdk.java.net/jdk/pull/6122
Re: RFR: 8275650: Problemlist java/io/File/createTempFile/SpecialTempFile.java for Windows 11 [v2]
> cc @ctornqvi Ivan Šipka 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 two additional commits since the last revision: - fixed OS identifier - 8274122: added to problem list - Changes: - all: https://git.openjdk.java.net/jdk/pull/6025/files - new: https://git.openjdk.java.net/jdk/pull/6025/files/55b0228c..931a9ea4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=6025=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6025=00-01 Stats: 20953 lines in 477 files changed: 16782 ins; 2674 del; 1497 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
RFR (CSR): 8202555: Double.toString(double) sometimes produces incorrect results
Hello, PR [1] and the accompanying article [2] have been subject to some positive reactions in the last couple of weeks. A fresh set of about 20 thousand additional hard test cases kindly provided by Paul Zimmermann of INRIA and other tests proposed by Guy Steele have been added to the code. The corresponding CSR [3] is in Finalized state for review. The proposed spec has been carefully written to uniquely define the outcomes and the code in the PR has been extensively tested to match the proposed spec. Behavioral backward compatibility has been a major goal for the CSR. In fact, in the vast majority of cases, the CSR and the current implementation agree on the outcomes. As the CSR is a prerequisite for the advancement of the PR, I beg everybody entitled (and interested) to review and approve it and/or discuss it further. Greetings Raffaello [1] https://github.com/openjdk/jdk/pull/3402 [2] https://drive.google.com/file/d/1luHhyQF9zKlM8yJ1nebU0OgVYhfC6CBN/view [3] https://bugs.openjdk.java.net/browse/JDK-8202555
Re: RFR: 8274750: java/io/File/GetXSpace.java failed: '/dev': 191488 != 190976
On Tue, 26 Oct 2021 19:00:44 GMT, Brian Burkhalter wrote: > Please consider this proposed change to ignore comparing the total size of > `/dev` as returned by `DF(1)` and `STAT(2)` on macOS. Alternative approaches would be to compare `File.getTotalSpace()` with `FileStore.getTotalSpace()` and / or have a tolerance on the comparison. - PR: https://git.openjdk.java.net/jdk/pull/6122
RFR: 8274750: java/io/File/GetXSpace.java failed: '/dev': 191488 != 190976
Please consider this proposed change to ignore comparing the total size of `/dev` as returned by `DF(1)` and `STAT(2)` on macOS. - Commit messages: - 8274750: java/io/File/GetXSpace.java failed: '/dev': 191488 != 190976 Changes: https://git.openjdk.java.net/jdk/pull/6122/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6122=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8274750 Stats: 8 lines in 1 file changed: 7 ins; 1 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/6122.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6122/head:pull/6122 PR: https://git.openjdk.java.net/jdk/pull/6122
Re: RFR: 8275863: Use encodeASCII for ASCII-compatible DoubleByte encodings
On Mon, 25 Oct 2021 10:16:23 GMT, Claes Redestad wrote: > Enhance ASCII-compatible `DoubleByte` encodings to make use of the > `StringCoding.implEncodeAsciiArray` intrinsic, which makes many such > `CharsetEncoder`s encode ASCII text at speeds comparable to most single-byte > encoders - and also more in line with how well these charsets behave when > calling `String.getBytes`: > > Before: > > Benchmark (size) (type) Mode Cnt Score Error > Units > CharsetEncodeDecode.encode 16384 ISO-8859-1 avgt 30 3.021 ± 0.120 > us/op > CharsetEncodeDecode.encode 16384 Shift-JIS avgt 30 47.793 ± 1.942 > us/op > CharsetEncodeDecode.encode 16384 GB2312 avgt 30 49.598 ± 2.006 > us/op > CharsetEncodeDecode.encode 16384 EUC-JP avgt 30 68.709 ± 5.019 > us/op > CharsetEncodeDecode.encode 16384 EUC-KR avgt 30 48.033 ± 1.651 > us/op > > > Patched: > > Benchmark (size) (type) Mode Cnt Score Error > Units > CharsetEncodeDecode.encode 16384 ISO-8859-1 avgt 30 2.856 ± 0.078 > us/op > CharsetEncodeDecode.encode 16384 Shift-JIS avgt 30 5.287 ± 0.209 > us/op > CharsetEncodeDecode.encode 16384 GB2312 avgt 30 5.490 ± 0.251 > us/op > CharsetEncodeDecode.encode 16384 EUC-JP avgt 30 7.657 ± 0.368 > us/op > CharsetEncodeDecode.encode 16384 EUC-KR avgt 30 5.718 ± 0.267 > us/op > > > The `isASCIICompatible` predicate on `DoubleByte` was added in JEP 254 for > the purpose of implementing such ASCII fast-paths, but is only used in what > is now `String.encodeWithEncoder` to speed up `String.getBytes(...)` > operations. > > Testing: tier1-3 Good work - Marked as reviewed by alanb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6102
Re: RFR: 8275863: Use encodeASCII for ASCII-compatible DoubleByte encodings
On Mon, 25 Oct 2021 10:16:23 GMT, Claes Redestad wrote: > Enhance ASCII-compatible `DoubleByte` encodings to make use of the > `StringCoding.implEncodeAsciiArray` intrinsic, which makes many such > `CharsetEncoder`s encode ASCII text at speeds comparable to most single-byte > encoders - and also more in line with how well these charsets behave when > calling `String.getBytes`: > > Before: > > Benchmark (size) (type) Mode Cnt Score Error > Units > CharsetEncodeDecode.encode 16384 ISO-8859-1 avgt 30 3.021 ± 0.120 > us/op > CharsetEncodeDecode.encode 16384 Shift-JIS avgt 30 47.793 ± 1.942 > us/op > CharsetEncodeDecode.encode 16384 GB2312 avgt 30 49.598 ± 2.006 > us/op > CharsetEncodeDecode.encode 16384 EUC-JP avgt 30 68.709 ± 5.019 > us/op > CharsetEncodeDecode.encode 16384 EUC-KR avgt 30 48.033 ± 1.651 > us/op > > > Patched: > > Benchmark (size) (type) Mode Cnt Score Error > Units > CharsetEncodeDecode.encode 16384 ISO-8859-1 avgt 30 2.856 ± 0.078 > us/op > CharsetEncodeDecode.encode 16384 Shift-JIS avgt 30 5.287 ± 0.209 > us/op > CharsetEncodeDecode.encode 16384 GB2312 avgt 30 5.490 ± 0.251 > us/op > CharsetEncodeDecode.encode 16384 EUC-JP avgt 30 7.657 ± 0.368 > us/op > CharsetEncodeDecode.encode 16384 EUC-KR avgt 30 5.718 ± 0.267 > us/op > > > The `isASCIICompatible` predicate on `DoubleByte` was added in JEP 254 for > the purpose of implementing such ASCII fast-paths, but is only used in what > is now `String.encodeWithEncoder` to speed up `String.getBytes(...)` > operations. > > Testing: tier1-3 Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6102
Re: RFR: 8275863: Use encodeASCII for ASCII-compatible DoubleByte encodings
On Mon, 25 Oct 2021 10:16:23 GMT, Claes Redestad wrote: > Enhance ASCII-compatible `DoubleByte` encodings to make use of the > `StringCoding.implEncodeAsciiArray` intrinsic, which makes many such > `CharsetEncoder`s encode ASCII text at speeds comparable to most single-byte > encoders - and also more in line with how well these charsets behave when > calling `String.getBytes`: > > Before: > > Benchmark (size) (type) Mode Cnt Score Error > Units > CharsetEncodeDecode.encode 16384 ISO-8859-1 avgt 30 3.021 ± 0.120 > us/op > CharsetEncodeDecode.encode 16384 Shift-JIS avgt 30 47.793 ± 1.942 > us/op > CharsetEncodeDecode.encode 16384 GB2312 avgt 30 49.598 ± 2.006 > us/op > CharsetEncodeDecode.encode 16384 EUC-JP avgt 30 68.709 ± 5.019 > us/op > CharsetEncodeDecode.encode 16384 EUC-KR avgt 30 48.033 ± 1.651 > us/op > > > Patched: > > Benchmark (size) (type) Mode Cnt Score Error > Units > CharsetEncodeDecode.encode 16384 ISO-8859-1 avgt 30 2.856 ± 0.078 > us/op > CharsetEncodeDecode.encode 16384 Shift-JIS avgt 30 5.287 ± 0.209 > us/op > CharsetEncodeDecode.encode 16384 GB2312 avgt 30 5.490 ± 0.251 > us/op > CharsetEncodeDecode.encode 16384 EUC-JP avgt 30 7.657 ± 0.368 > us/op > CharsetEncodeDecode.encode 16384 EUC-KR avgt 30 5.718 ± 0.267 > us/op > > > The `isASCIICompatible` predicate on `DoubleByte` was added in JEP 254 for > the purpose of implementing such ASCII fast-paths, but is only used in what > is now `String.encodeWithEncoder` to speed up `String.getBytes(...)` > operations. > > Testing: tier1-3 Looks good. - Marked as reviewed by naoto (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6102
Re: RFR: 8193682: Infinite loop in ZipOutputStream.close() [v4]
On Tue, 26 Oct 2021 06:30:39 GMT, Ravi Reddy wrote: >> Hi all, >> >> Please review this fix for Infinite loop in ZipOutputStream.close(). >> The main issue here is when ever there is an exception during close >> operations on GZip we are not setting the deflator to a finished state which >> is leading to an infinite loop when we try writing on the same GZip >> instance( since we use while(!def.finished()) inside the write operation). >> >> Thanks, >> Ravi > > Ravi Reddy has updated the pull request incrementally with one additional > commit since the last revision: > > 8193682 : Infinite loop in ZipOutputStream.close() I think overall this looks good. Thank you for continuing to work on this. I do believe it would be worth adding a CSR just to document the behavior realizing that it was always left undefined in the past Please also add a comment to each test describing its intent test/jdk/java/util/zip/GZIP/GZipLoopTest.java line 41: > 39: public class GZipLoopTest { > 40: private static final int FINISH_NUM = 512; > 41: private static OutputStream outStream = new OutputStream() { Please add a comment describing the intent of outstream and for FINISH_NUM. You might also consider a different name vs FINISH_NUM. Perhaps the comment will clarify this test/jdk/java/util/zip/GZIP/GZipLoopTest.java line 57: > 55: @Test > 56: public void testGZipClose() throws IOException { > 57: rand.nextBytes(b); You could possibly consider using a BeforeTest or BeforeMethod if you choose to reduce redundancy. No biggie otherwise. test/jdk/java/util/zip/GZIP/GZipLoopTest.java line 65: > 63: } catch (IOException e) { > 64: //expected > 65: } For the above if an IOException is expected, should this be tested via assertThrows()? - PR: https://git.openjdk.java.net/jdk/pull/5522
Re: RFR: 8271820: Implementation of JEP 416: Reimplement Core Reflection with Method Handle [v15]
> 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 Mandy Chung has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 40 commits: - Fall back to the VM native reflection support if method handle cannot be created - fix bug id in test - Merge - 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 - Separate paramFlgas into paramCount and flags fields - Minor cleanup. Improve javadoc in CallerSensitiveAdapter - Fix left-over assignment - Remove duplicated microbenchmarks - Avoid pitfall with unwanted inlining in some cases. Also remove boxing/unboxing to focus on the invocation cost - ... and 30 more: https://git.openjdk.java.net/jdk/compare/97d3280e...64738bb2 - Changes: https://git.openjdk.java.net/jdk/pull/5027/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5027=14 Stats: 6409 lines in 78 files changed: 5864 ins; 317 del; 228 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
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v9]
On Tue, 26 Oct 2021 16:24:48 GMT, Aleksei Efimov wrote: >> This change implements a new service provider interface for host name and >> address resolution, so that java.net.InetAddress API can make use of >> resolvers other than the platform's built-in resolver. >> >> The following API classes are added to `java.net.spi` package to facilitate >> this: >> - `InetAddressResolverProvider` - abstract class defining a service, and >> is, essentially, a factory for `InetAddressResolver` resolvers. >> - `InetAddressResolverProvider.Configuration ` - an interface describing the >> platform's built-in configuration for resolution operations that could be >> used to bootstrap a resolver construction, or to implement partial >> delegation of lookup operations. >> - `InetAddressResolver` - an interface that defines methods for the >> fundamental forward and reverse lookup operations. >> - `InetAddressResolver.LookupPolicy` - a class whose instances describe the >> characteristics of one forward lookup operation. >> >> More details in [JEP-418](https://openjdk.java.net/jeps/418). >> >> Testing: new and existing `tier1:tier3` tests > > Aleksei Efimov has updated the pull request incrementally with one additional > commit since the last revision: > > Add @ throws NPE to hosts file resolver javadoc Marked as reviewed by dfuchs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5822
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v8]
On Tue, 26 Oct 2021 15:04:54 GMT, Daniel Fuchs wrote: >> Aleksei Efimov 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 14 additional >> commits since the last revision: >> >> - Changes to address review comments >> - Update tests to address SM deprecation >> - Merge branch 'master' into JDK-8244202_JEP418_impl >> - More javadoc updates to API classes >> - Review updates + move resolver docs to the provider class (CSR update to >> follow) >> - Change InetAddressResolver method names >> - Remove no longer used import from IPSupport >> - Rename IPSupport.hasAddress and update it to use SocketChannel.open >> - Address review comments: javadoc + code cleanup >> - Address resolver bootstraping issue >> - ... and 4 more: >> https://git.openjdk.java.net/jdk/compare/1bbecc93...1378686b > > src/java.base/share/classes/java/net/InetAddress.java line 1151: > >> 1149: >> 1150: Objects.requireNonNull(host); >> 1151: Objects.requireNonNull(lookupPolicy); > > for consistency we could add `@throws NullPointerException` to the API doc of > that method - since it seems to have everything else... Added `@throws NullPointerException` to the hosts file resolver API docs: 2ca78ba98dc81cd6cc44b53dc7d56a6ae42c2736 - PR: https://git.openjdk.java.net/jdk/pull/5822
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v9]
> This change implements a new service provider interface for host name and > address resolution, so that java.net.InetAddress API can make use of > resolvers other than the platform's built-in resolver. > > The following API classes are added to `java.net.spi` package to facilitate > this: > - `InetAddressResolverProvider` - abstract class defining a service, and is, > essentially, a factory for `InetAddressResolver` resolvers. > - `InetAddressResolverProvider.Configuration ` - an interface describing the > platform's built-in configuration for resolution operations that could be > used to bootstrap a resolver construction, or to implement partial delegation > of lookup operations. > - `InetAddressResolver` - an interface that defines methods for the > fundamental forward and reverse lookup operations. > - `InetAddressResolver.LookupPolicy` - a class whose instances describe the > characteristics of one forward lookup operation. > > More details in [JEP-418](https://openjdk.java.net/jeps/418). > > Testing: new and existing `tier1:tier3` tests Aleksei Efimov has updated the pull request incrementally with one additional commit since the last revision: Add @ throws NPE to hosts file resolver javadoc - Changes: - all: https://git.openjdk.java.net/jdk/pull/5822/files - new: https://git.openjdk.java.net/jdk/pull/5822/files/1378686b..2ca78ba9 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5822=08 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5822=07-08 Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/5822.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5822/head:pull/5822 PR: https://git.openjdk.java.net/jdk/pull/5822
Re: RFR: 8271820: Implementation of JEP 416: Reimplement Core Reflection with Method Handle [v14]
On Mon, 25 Oct 2021 22:39:47 GMT, intrigus wrote: > Question: Can someone confirm that `Method.invoke` will still work with 255 > parameters after this PR gets merged? Thanks for the test case. For the case when method handles cannot be created due to the arity limit, it can fall back to the VM native reflection support. I have a fix for it. - PR: https://git.openjdk.java.net/jdk/pull/5027
Re: RFR: 8275293: A change done with JDK-8268764 mismatches the java.rmi.server.ObjID.hashCode spec
It is a non-goal to replicate all of the JCK test coverage in the regression test suite. -Joe On 10/25/2021 7:17 PM, Sergey Bylokhov wrote: On Fri, 15 Oct 2021 07:17:52 GMT, Сергей Цыпанов wrote: It looks like we cannot use `Long.hashCode(long)` for `java.rmi.server.ObjID.hashCode()` due to specification: https://docs.oracle.com/en/java/javase/17/docs/api/java.rmi/java/rmi/server/ObjID.html#hashCode(). I think this one was missed due to the absence of the coverage in the jtreg test suite, and some people have no access to the jck to run it in advance. - PR: https://git.openjdk.java.net/jdk/pull/5963
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v8]
On Tue, 26 Oct 2021 12:52:58 GMT, Aleksei Efimov wrote: >> This change implements a new service provider interface for host name and >> address resolution, so that java.net.InetAddress API can make use of >> resolvers other than the platform's built-in resolver. >> >> The following API classes are added to `java.net.spi` package to facilitate >> this: >> - `InetAddressResolverProvider` - abstract class defining a service, and >> is, essentially, a factory for `InetAddressResolver` resolvers. >> - `InetAddressResolverProvider.Configuration ` - an interface describing the >> platform's built-in configuration for resolution operations that could be >> used to bootstrap a resolver construction, or to implement partial >> delegation of lookup operations. >> - `InetAddressResolver` - an interface that defines methods for the >> fundamental forward and reverse lookup operations. >> - `InetAddressResolver.LookupPolicy` - a class whose instances describe the >> characteristics of one forward lookup operation. >> >> More details in [JEP-418](https://openjdk.java.net/jeps/418). >> >> Testing: new and existing `tier1:tier3` tests > > Aleksei Efimov 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 14 additional > commits since the last revision: > > - Changes to address review comments > - Update tests to address SM deprecation > - Merge branch 'master' into JDK-8244202_JEP418_impl > - More javadoc updates to API classes > - Review updates + move resolver docs to the provider class (CSR update to > follow) > - Change InetAddressResolver method names > - Remove no longer used import from IPSupport > - Rename IPSupport.hasAddress and update it to use SocketChannel.open > - Address review comments: javadoc + code cleanup > - Address resolver bootstraping issue > - ... and 4 more: > https://git.openjdk.java.net/jdk/compare/1009...1378686b Marked as reviewed by dfuchs (Reviewer). src/java.base/share/classes/java/net/InetAddress.java line 1151: > 1149: > 1150: Objects.requireNonNull(host); > 1151: Objects.requireNonNull(lookupPolicy); for consistency we could add `@throws NullPointerException` to the API doc of that method - since it seems to have everything else... - PR: https://git.openjdk.java.net/jdk/pull/5822
Re: RFR: 8270490: Charset.forName() taking fallback default value [v4]
On Sat, 23 Oct 2021 22:13:35 GMT, Naoto Sato wrote: >> During the review of JEP 400, a proposal to provide an overloaded method to >> `Charset.forName()` was suggested >> [[1]](https://github.com/openjdk/jdk/pull/4733#discussion_r669693954). This >> PR is to implement the proposal. A CSR is also drafted as >> https://bugs.openjdk.java.net/browse/JDK-8275348 > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Reflecting review comments Marked as reviewed by dfuchs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6045
Re: RFR: 8270490: Charset.forName() taking fallback default value [v4]
On Tue, 26 Oct 2021 12:59:11 GMT, Naoto Sato wrote: >> src/java.base/share/classes/java/io/Console.java line 590: >> >>> 588: if (cs == null) { >>> 589: cs = Charset.forName(StaticProperty.nativeEncoding(), >>> 590: Charset.defaultCharset()); >> >> I assume that `StaticProperty.nativeEncoding()` will never be `null`? >> Otherwise an IAE would be thrown here where previously >> `Charset.defaultCharset()` would be used. > > Yes. `StaticProperty.nativeEncoding()` caches the value to `native.encoding` > system property. The value is not optional, so it should be considered an > error if `StaticProperty.nativeEncoding()` returned `null`. > https://download.java.net/java/early_access/jdk18/docs/api/java.base/java/lang/System.html#native.encoding Thanks for the clarification. - PR: https://git.openjdk.java.net/jdk/pull/6045
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v8]
On Tue, 26 Oct 2021 12:52:58 GMT, Aleksei Efimov wrote: >> This change implements a new service provider interface for host name and >> address resolution, so that java.net.InetAddress API can make use of >> resolvers other than the platform's built-in resolver. >> >> The following API classes are added to `java.net.spi` package to facilitate >> this: >> - `InetAddressResolverProvider` - abstract class defining a service, and >> is, essentially, a factory for `InetAddressResolver` resolvers. >> - `InetAddressResolverProvider.Configuration ` - an interface describing the >> platform's built-in configuration for resolution operations that could be >> used to bootstrap a resolver construction, or to implement partial >> delegation of lookup operations. >> - `InetAddressResolver` - an interface that defines methods for the >> fundamental forward and reverse lookup operations. >> - `InetAddressResolver.LookupPolicy` - a class whose instances describe the >> characteristics of one forward lookup operation. >> >> More details in [JEP-418](https://openjdk.java.net/jeps/418). >> >> Testing: new and existing `tier1:tier3` tests > > Aleksei Efimov 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 14 additional > commits since the last revision: > > - Changes to address review comments > - Update tests to address SM deprecation > - Merge branch 'master' into JDK-8244202_JEP418_impl > - More javadoc updates to API classes > - Review updates + move resolver docs to the provider class (CSR update to > follow) > - Change InetAddressResolver method names > - Remove no longer used import from IPSupport > - Rename IPSupport.hasAddress and update it to use SocketChannel.open > - Address review comments: javadoc + code cleanup > - Address resolver bootstraping issue > - ... and 4 more: > https://git.openjdk.java.net/jdk/compare/c2db4e04...1378686b Thanks for the updates to the API docs, you've addressed all my comments. - Marked as reviewed by alanb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5822
Re: RFR: 8270490: Charset.forName() taking fallback default value [v4]
On Tue, 26 Oct 2021 10:42:49 GMT, Daniel Fuchs wrote: >> Naoto Sato has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Reflecting review comments > > src/java.base/share/classes/java/io/Console.java line 590: > >> 588: if (cs == null) { >> 589: cs = Charset.forName(StaticProperty.nativeEncoding(), >> 590: Charset.defaultCharset()); > > I assume that `StaticProperty.nativeEncoding()` will never be `null`? > Otherwise an IAE would be thrown here where previously > `Charset.defaultCharset()` would be used. Yes. `StaticProperty.nativeEncoding()` caches the value to `native.encoding` system property. The value is not optional, so it should be considered an error if `StaticProperty.nativeEncoding()` returned `null`. https://download.java.net/java/early_access/jdk18/docs/api/java.base/java/lang/System.html#native.encoding - PR: https://git.openjdk.java.net/jdk/pull/6045
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v5]
On Sat, 23 Oct 2021 06:33:52 GMT, Alan Bateman wrote: >> src/java.base/share/classes/java/net/spi/InetAddressResolverProvider.java >> line 52: >> >>> 50: /** >>> 51: * Initialise and return the {@link InetAddressResolver} provided by >>> 52: * this provider. This method is called by {@link InetAddress} when >> >> "the InetAddressResolver" suggests there is just one instance. That is >> probably the case but I don't think you want to be restricted to that. > > Initialise -> Initialize to be consistent with other usages that use US > spelling. Thanks, changed in 1378686becfcbf18793556de8381b5ebcd79698d - PR: https://git.openjdk.java.net/jdk/pull/5822
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v7]
On Tue, 26 Oct 2021 12:49:30 GMT, Aleksei Efimov wrote: >> src/java.base/share/classes/java/net/spi/InetAddressResolverProvider.java >> line 45: >> >>> 43: * system-wide resolver instance, which is used by >>> 44: * >> href="{@docRoot}/java.base/java/net/InetAddress.html#host-name-resolution"> >>> 45: * InetAddress. >> >> I think we should prune some some of the text from the class description to >> avoid repetition. Here's a suggestion: >> >> 1. Add the following immediately after the sentence "A given innovation ..." >> "It is set after the VM is fully initialized and when an invocation of a >> method in InetAddress class triggers the first lookup operation. >> >> 2. Replace the text in L47-65 with: >> "A resolver provider is located and loaded by InetAddress to create the >> systwm-wide resolver as follows: >> >> 3. Replace the remaining three usages of "install" with "set". > > Thanks, changed as suggested: 1378686becfcbf18793556de8381b5ebcd79698d Pruned `InetAddressResolverProvider` class-level doc as suggested: 1378686becfcbf18793556de8381b5ebcd79698d - PR: https://git.openjdk.java.net/jdk/pull/5822
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v7]
On Sat, 23 Oct 2021 06:19:46 GMT, Alan Bateman wrote: >> Aleksei Efimov has updated the pull request incrementally with one >> additional commit since the last revision: >> >> More javadoc updates to API classes > > src/java.base/share/classes/java/net/InetAddress.java line 169: > >> 167: * Access Protocol (LDAP). >> 168: * The particular naming services that the built-in resolver uses by >> default >> 169: * depend on the configuration of the local machine. > > depend -> depends Thanks, changed in 1378686becfcbf18793556de8381b5ebcd79698d > src/java.base/share/classes/java/net/spi/InetAddressResolver.java line 38: > >> 36: * deployed as a > href="InetAddressResolverProvider.html#system-wide-resolver"> >> 37: * system-wide resolver. {@link InetAddress} delegates all lookup >> requests to >> 38: * the deployed system-wide resolver instance. > > I think the class description would be a bit clearer if we drop sentence 2 > because it overlaps with paragraph 2. I think we can adjust sentence 3 to > "InetAddress delegates all lookup operations to the system-wide resolver" and > it will all flow nicely. Thanks, changed as suggested in 1378686becfcbf18793556de8381b5ebcd79698d > src/java.base/share/classes/java/net/spi/InetAddressResolver.java line 88: > >> 86: * to a valid IP address length >> 87: */ >> 88: String lookupByAddress(byte[] addr) throws UnknownHostException; > > I assume this throws NPE if addr is null. 1378686becfcbf18793556de8381b5ebcd79698d: added @ throws NPE javadoc, also added additional checks for `null` parameter values into the built-in and hosts file resolver implementations. > src/java.base/share/classes/java/net/spi/InetAddressResolverProvider.java > line 45: > >> 43: * system-wide resolver instance, which is used by >> 44: * > href="{@docRoot}/java.base/java/net/InetAddress.html#host-name-resolution"> >> 45: * InetAddress. > > I think we should prune some some of the text from the class description to > avoid repetition. Here's a suggestion: > > 1. Add the following immediately after the sentence "A given innovation ..." > "It is set after the VM is fully initialized and when an invocation of a > method in InetAddress class triggers the first lookup operation. > > 2. Replace the text in L47-65 with: > "A resolver provider is located and loaded by InetAddress to create the > systwm-wide resolver as follows: > > 3. Replace the remaining three usages of "install" with "set". Thanks, changed as suggested: 1378686becfcbf18793556de8381b5ebcd79698d - PR: https://git.openjdk.java.net/jdk/pull/5822
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v8]
> This change implements a new service provider interface for host name and > address resolution, so that java.net.InetAddress API can make use of > resolvers other than the platform's built-in resolver. > > The following API classes are added to `java.net.spi` package to facilitate > this: > - `InetAddressResolverProvider` - abstract class defining a service, and is, > essentially, a factory for `InetAddressResolver` resolvers. > - `InetAddressResolverProvider.Configuration ` - an interface describing the > platform's built-in configuration for resolution operations that could be > used to bootstrap a resolver construction, or to implement partial delegation > of lookup operations. > - `InetAddressResolver` - an interface that defines methods for the > fundamental forward and reverse lookup operations. > - `InetAddressResolver.LookupPolicy` - a class whose instances describe the > characteristics of one forward lookup operation. > > More details in [JEP-418](https://openjdk.java.net/jeps/418). > > Testing: new and existing `tier1:tier3` tests Aleksei Efimov 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 14 additional commits since the last revision: - Changes to address review comments - Update tests to address SM deprecation - Merge branch 'master' into JDK-8244202_JEP418_impl - More javadoc updates to API classes - Review updates + move resolver docs to the provider class (CSR update to follow) - Change InetAddressResolver method names - Remove no longer used import from IPSupport - Rename IPSupport.hasAddress and update it to use SocketChannel.open - Address review comments: javadoc + code cleanup - Address resolver bootstraping issue - ... and 4 more: https://git.openjdk.java.net/jdk/compare/a8495826...1378686b - Changes: - all: https://git.openjdk.java.net/jdk/pull/5822/files - new: https://git.openjdk.java.net/jdk/pull/5822/files/fa655be2..1378686b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5822=07 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5822=06-07 Stats: 32082 lines in 838 files changed: 22928 ins; 6060 del; 3094 mod Patch: https://git.openjdk.java.net/jdk/pull/5822.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5822/head:pull/5822 PR: https://git.openjdk.java.net/jdk/pull/5822
Integrated: 8275767: JDK source code contains redundant boolean operations in jdk.charsets
On Mon, 25 Oct 2021 16:08:29 GMT, Naoto Sato wrote: > Trivial clean-up. This pull request has now been integrated. Changeset: 63e0f344 Author:Naoto Sato URL: https://git.openjdk.java.net/jdk/commit/63e0f344e9a2da135c76caff11e437dfc40408a6 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod 8275767: JDK source code contains redundant boolean operations in jdk.charsets Reviewed-by: lancea, rriggs, iris - PR: https://git.openjdk.java.net/jdk/pull/6110
Re: RFR: 8193682: Infinite loop in ZipOutputStream.close() [v4]
On Tue, 26 Oct 2021 06:30:39 GMT, Ravi Reddy wrote: >> Hi all, >> >> Please review this fix for Infinite loop in ZipOutputStream.close(). >> The main issue here is when ever there is an exception during close >> operations on GZip we are not setting the deflator to a finished state which >> is leading to an infinite loop when we try writing on the same GZip >> instance( since we use while(!def.finished()) inside the write operation). >> >> Thanks, >> Ravi > > Ravi Reddy has updated the pull request incrementally with one additional > commit since the last revision: > > 8193682 : Infinite loop in ZipOutputStream.close() src/java.base/share/classes/java/util/zip/DeflaterOutputStream.java line 247: > 245:if (usesDefaultDeflater) > 246: def.end(); > 247:throw e; The formatting is a bit wonky but I think this version is the best so far. It does mean that the stream state is undefined when close throws but this has always been the case. - PR: https://git.openjdk.java.net/jdk/pull/5522
Integrated: 8275137: jdk.unsupported/sun.reflect.ReflectionFactory.readObjectNoDataForSerialization uses wrong signature
On Thu, 14 Oct 2021 14:44:34 GMT, Julia Boes wrote: > sun.reflect.ReflectionFactory provides MethodHandles for the various > serialization methods, it is a critical internal API in the jdk.unsupported > module (see JEP 260 [1]) that may be used by 3rd party serialization > libraries. > > One of these serialization methods is readObjectNoData [2]: > > ```private void readObjectNoData() throws ObjectStreamException;``` > > The issue: The method that returns the matching handle, > sun.reflect.ReflectionFactory.readObjectNoDataForSerialization, uses an > erroneous signature `readObjectNoData(ObjectInputStream)` - note the > superfluous parameter. > > This change updates the specification and fixes the implementation in > java.base/jdk.internal.reflect.ReflectionFactory. > > Testing: tier 1-3 > > [1] https://openjdk.java.net/jeps/260 > [2] > https://docs.oracle.com/en/java/javase/15/docs/specs/serialization/input.html#the-readobjectnodata-method This pull request has now been integrated. Changeset: 4961373a Author:Julia Boes URL: https://git.openjdk.java.net/jdk/commit/4961373a676126cd557f92a2e7bbc8c66b2976b1 Stats: 28 lines in 3 files changed: 4 ins; 6 del; 18 mod 8275137: jdk.unsupported/sun.reflect.ReflectionFactory.readObjectNoDataForSerialization uses wrong signature Reviewed-by: dfuchs - PR: https://git.openjdk.java.net/jdk/pull/5951
Re: RFR: 8271820: Implementation of JEP 416: Reimplement Core Reflection with Method Handle [v14]
On Mon, 18 Oct 2021 16:10:21 GMT, Mandy Chung wrote: >> 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 > > Mandy Chung has updated the pull request incrementally with one additional > commit since the last revision: > > Separate paramFlgas into paramCount and flags fields (I've looked at the changed code as someone that is NOT familiar with the implementation of method handles and reflection. The code did not answer my question.) Is it still possible to reflectively invoke methods with 127 * 2 (long) + 1 (int) = 255 parameters? The following code tests this once using `Method.invoke` and another time it tries to test it using `MethodHandles.lookup().unreflect()`. (I'm assuming (!) that the new code in this PR is pretty much "equivalent" to `MethodHandles.lookup().unreflect()`) The test using method handles fails with an (expected) IAE. Question: Can someone confirm that `Method.invoke` will still work with 255 parameters after this PR gets merged? java.lang.IllegalArgumentException: bad parameter count 256 at java.base/java.lang.invoke.MethodHandleStatics.newIllegalArgumentException(MethodHandleStatics.java:134) at java.base/java.lang.invoke.MethodType.checkSlotCount(MethodType.java:229) at java.base/java.lang.invoke.MethodType.insertParameterTypes(MethodType.java:440) at java.base/java.lang.invoke.MethodType.appendParameterTypes(MethodType.java:464) at java.base/java.lang.invoke.DirectMethodHandle.makePreparedLambdaForm(DirectMethodHandle.java:257) at java.base/java.lang.invoke.DirectMethodHandle.preparedLambdaForm(DirectMethodHandle.java:234) at java.base/java.lang.invoke.DirectMethodHandle.preparedLambdaForm(DirectMethodHandle.java:219) at java.base/java.lang.invoke.DirectMethodHandle.preparedLambdaForm(DirectMethodHandle.java:228) at java.base/java.lang.invoke.DirectMethodHandle.make(DirectMethodHandle.java:108) at java.base/java.lang.invoke.MethodHandles$Lookup.getDirectMethodCommon(MethodHandles.java:3988) at java.base/java.lang.invoke.MethodHandles$Lookup.getDirectMethodNoSecurityManager(MethodHandles.java:3944) at java.base/java.lang.invoke.MethodHandles$Lookup.unreflect(MethodHandles.java:3351) at ReflectionParameterCountTest.lambda$0(ReflectionParameterCountTest.java:42) at org.junit.jupiter.api.AssertDoesNotThrow.assertDoesNotThrow(AssertDoesNotThrow.java:50) import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodHandles; import java.lang.reflect.Method; import org.junit.jupiter.api.Test; public class ReflectionParameterCountTest { @Test public void testReflectionMaxParameterCount() throws Throwable { Method m = this.getClass().getMethod("f", long.class, long.class, long.class, long.class, long.class, long.class, long.class, long.class, long.class, long.class, long.class, long.class, long.class, long.class, long.class, long.class, long.class, long.class, long.class, long.class, long.class, long.class, long.class, long.class, long.class, long.class, long.class, long.class, long.class, long.class, long.class, long.class, long.class, long.class, long.class, long.class,
Re: RFR: 8270490: Charset.forName() taking fallback default value [v4]
On Sat, 23 Oct 2021 22:13:35 GMT, Naoto Sato wrote: >> During the review of JEP 400, a proposal to provide an overloaded method to >> `Charset.forName()` was suggested >> [[1]](https://github.com/openjdk/jdk/pull/4733#discussion_r669693954). This >> PR is to implement the proposal. A CSR is also drafted as >> https://bugs.openjdk.java.net/browse/JDK-8275348 > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Reflecting review comments src/java.base/share/classes/java/io/Console.java line 590: > 588: if (cs == null) { > 589: cs = Charset.forName(StaticProperty.nativeEncoding(), > 590: Charset.defaultCharset()); I assume that `StaticProperty.nativeEncoding()` will never be `null`? Otherwise an IAE would be thrown here where previously `Charset.defaultCharset()` would be used. - PR: https://git.openjdk.java.net/jdk/pull/6045
Re: RFR: 8193682: Infinite loop in ZipOutputStream.close() [v4]
On Tue, 26 Oct 2021 06:30:39 GMT, Ravi Reddy wrote: >> Hi all, >> >> Please review this fix for Infinite loop in ZipOutputStream.close(). >> The main issue here is when ever there is an exception during close >> operations on GZip we are not setting the deflator to a finished state which >> is leading to an infinite loop when we try writing on the same GZip >> instance( since we use while(!def.finished()) inside the write operation). >> >> Thanks, >> Ravi > > Ravi Reddy has updated the pull request incrementally with one additional > commit since the last revision: > > 8193682 : Infinite loop in ZipOutputStream.close() Hi , I have made the changes in fix , I think changing existing behavior in deflate() will need a spec change. With this fix , I'm closing deflater on exception scenarios in finish method of GZipOutputStream and close method of DeflaterOutputStream. Even though the document for finsih/close methods does not clearly specifies if the deflater will be closed or not , the current behaviour of these methods does close the deflater whenever finish/close are called. \ Thanks, Ravi - PR: https://git.openjdk.java.net/jdk/pull/5522
Re: RFR: 8193682: Infinite loop in ZipOutputStream.close() [v4]
> Hi all, > > Please review this fix for Infinite loop in ZipOutputStream.close(). > The main issue here is when ever there is an exception during close > operations on GZip we are not setting the deflator to a finished state which > is leading to an infinite loop when we try writing on the same GZip instance( > since we use while(!def.finished()) inside the write operation). > > Thanks, > Ravi Ravi Reddy has updated the pull request incrementally with one additional commit since the last revision: 8193682 : Infinite loop in ZipOutputStream.close() - Changes: - all: https://git.openjdk.java.net/jdk/pull/5522/files - new: https://git.openjdk.java.net/jdk/pull/5522/files/d18eb3c1..5f1922bf Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5522=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5522=02-03 Stats: 26 lines in 2 files changed: 12 ins; 7 del; 7 mod Patch: https://git.openjdk.java.net/jdk/pull/5522.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5522/head:pull/5522 PR: https://git.openjdk.java.net/jdk/pull/5522