Re: RFR: JDK-8275703: System.loadLibrary fails on Big Sur for libraries hidden from filesystem

2021-10-26 Thread Jaikiran Pai
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

2021-10-26 Thread Jaikiran Pai
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

2021-10-26 Thread Jaikiran Pai
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

2021-10-26 Thread David Holmes
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

2021-10-26 Thread Mandy Chung
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]

2021-10-26 Thread Brian Burkhalter
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]

2021-10-26 Thread Igor Ignatyev
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]

2021-10-26 Thread Brian Burkhalter
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

2021-10-26 Thread Brian Burkhalter
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

2021-10-26 Thread Naoto Sato
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]

2021-10-26 Thread Ivan Šipka
> 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

2021-10-26 Thread Raffaello Giulietti

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

2021-10-26 Thread Brian Burkhalter
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

2021-10-26 Thread Brian Burkhalter
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

2021-10-26 Thread Alan Bateman
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

2021-10-26 Thread Roger Riggs
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

2021-10-26 Thread Naoto Sato
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]

2021-10-26 Thread Lance Andersen
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]

2021-10-26 Thread Mandy Chung
> 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]

2021-10-26 Thread Daniel Fuchs
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]

2021-10-26 Thread Aleksei Efimov
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]

2021-10-26 Thread Aleksei Efimov
> 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]

2021-10-26 Thread Mandy Chung
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

2021-10-26 Thread Joseph D. Darcy
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]

2021-10-26 Thread Daniel Fuchs
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]

2021-10-26 Thread Daniel Fuchs
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]

2021-10-26 Thread Daniel Fuchs
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]

2021-10-26 Thread Alan Bateman
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]

2021-10-26 Thread Naoto Sato
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]

2021-10-26 Thread Aleksei Efimov
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]

2021-10-26 Thread Aleksei Efimov
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]

2021-10-26 Thread Aleksei Efimov
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]

2021-10-26 Thread Aleksei Efimov
> 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

2021-10-26 Thread Naoto Sato
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]

2021-10-26 Thread Alan Bateman
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

2021-10-26 Thread Julia Boes
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]

2021-10-26 Thread intrigus
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]

2021-10-26 Thread Daniel Fuchs
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]

2021-10-26 Thread Ravi Reddy
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]

2021-10-26 Thread Ravi Reddy
> 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