Re: RFR: 8287104: AddressChangeListener thread inherits CCL and can cause memory leak for webapp-servers [v3]
On Wed, 25 May 2022 09:30:46 GMT, Jaikiran Pai wrote: >> Can I please get a review of this change which addresses >> https://bugs.openjdk.java.net/browse/JDK-8287104? >> >> The change in this commit now uses an `InnocuousThread` to create a thread >> with `null` context classloader. The `Runnable` task of this thread just >> invokes a native method through JNI to be notified of IP addresses change of >> the host. As such any specific thread context classloader isn't necessary in >> this thread. >> >> Additionally, this commit does some minor changes like making the `lock` >> member variable `final` and also marking the `changed` member variable as >> `volatile`. These changes aren't necessary for this fix, but I think would >> be good to have while we are changing this part of the code. >> >> Finally, the thread that we create here, now has a specific name >> `Net-address-change-listener` instead of the usual system wide >> auto-generated name. >> >> No new tests have been added for this change. Existing tier1, tier2 and >> tier3 tests have been run and no related failures have been noticed. > > Jaikiran Pai has updated the pull request incrementally with one additional > commit since the last revision: > > No need for volatile as spotted by Daniel Marked as reviewed by aefimov (Committer). - PR: https://git.openjdk.java.net/jdk/pull/8827
Re: RFR: 8287104: AddressChangeListener thread inherits CCL and can cause memory leak for webapp-servers [v2]
On Mon, 23 May 2022 12:27:39 GMT, Jaikiran Pai wrote: >> Can I please get a review of this change which addresses >> https://bugs.openjdk.java.net/browse/JDK-8287104? >> >> The change in this commit now uses an `InnocuousThread` to create a thread >> with `null` context classloader. The `Runnable` task of this thread just >> invokes a native method through JNI to be notified of IP addresses change of >> the host. As such any specific thread context classloader isn't necessary in >> this thread. >> >> Additionally, this commit does some minor changes like making the `lock` >> member variable `final` and also marking the `changed` member variable as >> `volatile`. These changes aren't necessary for this fix, but I think would >> be good to have while we are changing this part of the code. >> >> Finally, the thread that we create here, now has a specific name >> `Net-address-change-listener` instead of the usual system wide >> auto-generated name. >> >> No new tests have been added for this change. Existing tier1, tier2 and >> tier3 tests have been run and no related failures have been noticed. > > Jaikiran Pai has updated the pull request incrementally with one additional > commit since the last revision: > > Aleksei's review suggestion - use a better Thread name Marked as reviewed by aefimov (Committer). - PR: https://git.openjdk.java.net/jdk/pull/8827
Re: RFR: JDK-8280498: jdk/java/net/Inet4Address/PingThis.java fails on AIX [v7]
On Fri, 6 May 2022 09:18:33 GMT, Michael Felt wrote: >> Michael Felt has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Changes per review > > - Thx all for your assistance and patience. > - (Hoping you will consider this for backport as well). Hi @aixtools, This is just a reminder that you can proceed with the integration of this PR :) - PR: https://git.openjdk.java.net/jdk/pull/7013
Re: RFR: 8287104: AddressChangeListener thread inherits CCL and can cause memory leak for webapp-servers
On Sun, 22 May 2022 08:20:21 GMT, Jaikiran Pai wrote: > Can I please get a review of this change which addresses > https://bugs.openjdk.java.net/browse/JDK-8287104? > > The change in this commit now uses an `InnocuousThread` to create a thread > with `null` context classloader. The `Runnable` task of this thread just > invokes a native method through JNI to be notified of IP addresses change of > the host. As such any specific thread context classloader isn't necessary in > this thread. > > Additionally, this commit does some minor changes like making the `lock` > member variable `final` and also marking the `changed` member variable as > `volatile`. These changes aren't necessary for this fix, but I think would be > good to have while we are changing this part of the code. > > Finally, the thread that we create here, now has a specific name > `Net-address-change-listener` instead of the usual system wide auto-generated > name. > > No new tests have been added for this change. Existing tier1, tier2 and tier3 > tests have been run and no related failures have been noticed. Hi @jaikiran, Thanks for the fix. Chages look good to me with one suggestion: Could we reflect in a thread name the fact that it is JNDI/DNS specific one? Right now it is not clear - one might think that it could be related to `InetAddress`/`NetworkInterface` or other networking classes. Maybe something like `Jndi-Dns-address-change-listener`? - Marked as reviewed by aefimov (Committer). PR: https://git.openjdk.java.net/jdk/pull/8827
Re: RFR: JDK-8280498: jdk/java/net/Inet4Address/PingThis.java fails on AIX [v7]
On Fri, 6 May 2022 09:18:33 GMT, Michael Felt wrote: >> Michael Felt has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Changes per review > > - Thx all for your assistance and patience. > - (Hoping you will consider this for backport as well). Hi @aixtools, The test runs fine on non-windows/non-aix platforms. The fix is ready for the integration: I will sponsor it after you issue `/integrate` [command](https://github.com/openjdk/jdk/pull/7013#issuecomment-165207). - PR: https://git.openjdk.java.net/jdk/pull/7013
Re: RFR: JDK-8280498: jdk/java/net/Inet4Address/PingThis.java fails on AIX [v7]
On Thu, 5 May 2022 12:55:57 GMT, Michael Felt wrote:
>> with IP "0.0.0.0"
>>
>> - it either does nothing and ping fails, or, in some virtual environments
>> is treated as the default route address.
>> - IPv6 support for ::1 is available since 1977; however, ::0 is not accepted
>> as a vaild psuedo IPv6 address. '::1' must be used instead.
>>
>> ping: bind: The socket name is not available on this system.
>> ping: bind: The socket name is not available on this system.
>> PING ::1: (::1): 56 data bytes
>> 64 bytes from ::1: icmp_seq=0 ttl=255 time=0.037 ms
>> 64 bytes from ::1: icmp_seq=1 ttl=255 time=0.045 ms
>>
>> --- ::1 ping statistics ---
>> 2 packets transmitted, 2 packets received, 0% packet loss
>> round-trip min/avg/max = 0/0/0 ms
>> PING ::1: (::1): 56 data bytes
>> 64 bytes from ::1: icmp_seq=0 ttl=255 time=0.052 ms
>> 64 bytes from ::1: icmp_seq=1 ttl=255 time=0.047 ms
>>
>> --- ::1 ping statistics ---
>> 2 packets transmitted, 2 packets received, 0% packet loss
>>
>>
>> A long commit message.
>>
>> This came to light because some systems failed with IPv4 (those that passed
>> replaced 0.0.0.0 with the default router. but most just fail - not
>> substituting
>> 0.0.0.0 with 127.0.0.1. However, InetAddress.getByName("") returns 127.0.0.1
>> which compares well with other platform's behavior.
>
> Michael Felt has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Changes per review
Latest changes look good to me. Will check that the test runs/not runs with our
CI and will sponsor it once I have the results.
-
Marked as reviewed by aefimov (Committer).
PR: https://git.openjdk.java.net/jdk/pull/7013
Re: RFR: JDK-8280498: jdk/java/net/Inet4Address/PingThis.java fails on AIX [v6]
On Sat, 30 Apr 2022 09:55:16 GMT, Michael Felt wrote:
>> with IP "0.0.0.0"
>>
>> - it either does nothing and ping fails, or, in some virtual environments
>> is treated as the default route address.
>> - IPv6 support for ::1 is available since 1977; however, ::0 is not accepted
>> as a vaild psuedo IPv6 address. '::1' must be used instead.
>>
>> ping: bind: The socket name is not available on this system.
>> ping: bind: The socket name is not available on this system.
>> PING ::1: (::1): 56 data bytes
>> 64 bytes from ::1: icmp_seq=0 ttl=255 time=0.037 ms
>> 64 bytes from ::1: icmp_seq=1 ttl=255 time=0.045 ms
>>
>> --- ::1 ping statistics ---
>> 2 packets transmitted, 2 packets received, 0% packet loss
>> round-trip min/avg/max = 0/0/0 ms
>> PING ::1: (::1): 56 data bytes
>> 64 bytes from ::1: icmp_seq=0 ttl=255 time=0.052 ms
>> 64 bytes from ::1: icmp_seq=1 ttl=255 time=0.047 ms
>>
>> --- ::1 ping statistics ---
>> 2 packets transmitted, 2 packets received, 0% packet loss
>>
>>
>> A long commit message.
>>
>> This came to light because some systems failed with IPv4 (those that passed
>> replaced 0.0.0.0 with the default router. but most just fail - not
>> substituting
>> 0.0.0.0 with 127.0.0.1. However, InetAddress.getByName("") returns 127.0.0.1
>> which compares well with other platform's behavior.
>
> Michael Felt has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Use @requires jtreg tag rather than check in code
test/jdk/java/net/Inet4Address/PingThis.java line 33:
> 31: * @library /test/lib
> 32: * @summary InetAddress.isReachable is returning false
> 33: * for InetAdress 0.0.0.0 and ::0
Typo: `InetAdress` -> `InetAddress`
test/jdk/java/net/Inet4Address/PingThis.java line 37:
> 35: * but does not respond as 127.0.0.1 (i.e., only responds
> 36: * when an external device reacts to 0.0.0.0)
> 37: */
This line should be removed to fix formatting of the comment section, ie
unbalanced `*/`
-
PR: https://git.openjdk.java.net/jdk/pull/7013
Re: RFR: JDK-8280498: jdk/java/net/Inet4Address/PingThis.java fails on AIX [v5]
On Fri, 29 Apr 2022 12:16:36 GMT, Michael Felt wrote:
>> with IP "0.0.0.0"
>>
>> - it either does nothing and ping fails, or, in some virtual environments
>> is treated as the default route address.
>> - IPv6 support for ::1 is available since 1977; however, ::0 is not accepted
>> as a vaild psuedo IPv6 address. '::1' must be used instead.
>>
>> ping: bind: The socket name is not available on this system.
>> ping: bind: The socket name is not available on this system.
>> PING ::1: (::1): 56 data bytes
>> 64 bytes from ::1: icmp_seq=0 ttl=255 time=0.037 ms
>> 64 bytes from ::1: icmp_seq=1 ttl=255 time=0.045 ms
>>
>> --- ::1 ping statistics ---
>> 2 packets transmitted, 2 packets received, 0% packet loss
>> round-trip min/avg/max = 0/0/0 ms
>> PING ::1: (::1): 56 data bytes
>> 64 bytes from ::1: icmp_seq=0 ttl=255 time=0.052 ms
>> 64 bytes from ::1: icmp_seq=1 ttl=255 time=0.047 ms
>>
>> --- ::1 ping statistics ---
>> 2 packets transmitted, 2 packets received, 0% packet loss
>>
>>
>> A long commit message.
>>
>> This came to light because some systems failed with IPv4 (those that passed
>> replaced 0.0.0.0 with the default router. but most just fail - not
>> substituting
>> 0.0.0.0 with 127.0.0.1. However, InetAddress.getByName("") returns 127.0.0.1
>> which compares well with other platform's behavior.
>
> Michael Felt has updated the pull request incrementally with one additional
> commit since the last revision:
>
> missing semi-colon
The changes looks ok to me given that the test can be compiled and executed
with no failures.
The following question being asked before ,but is there a reason why we check
platform in the test code, instead of using [requires jtreg
tag](https://openjdk.java.net/jtreg/tag-spec.html#requires_names) ?
The following requires tag could be used as a replacement for the proposed
change:
* @requires (os.family != "windows" & os.family != "aix")
-
Marked as reviewed by aefimov (Committer).
PR: https://git.openjdk.java.net/jdk/pull/7013
Integrated: 8283772: Make sun.net.dns.ResolverConfiguration sealed
On Mon, 28 Mar 2022 21:44:36 GMT, Aleksei Efimov wrote: > The following fix seals `sun.net.dns.ResolverConfiguration` abstract class. > `sun.net.dns.ResolverConfigurationImpl` is the only permitted subclass which > has two O/S specific implementations: for `Windows` and `Unix` architectures. > Both of them are marked as `final`. > > Testing: `jdk-tier1`, `jdk-tier2`, `jdk-tier3` and `java.naming` JCK tests > show no failures with the change. This pull request has now been integrated. Changeset: 95913067 Author:Aleksei Efimov URL: https://git.openjdk.java.net/jdk/commit/9591306760ffa0725977db8d3bb66758baeafbf6 Stats: 6 lines in 3 files changed: 0 ins; 0 del; 6 mod 8283772: Make sun.net.dns.ResolverConfiguration sealed Reviewed-by: jpai, dfuchs - PR: https://git.openjdk.java.net/jdk/pull/8006
RFR: 8283772: Make sun.net.dns.ResolverConfiguration sealed
The following fix seals `sun.net.dns.ResolverConfiguration` abstract class. `sun.net.dns.ResolverConfigurationImpl` is the only permitted subclass which has two O/S specific implementations: for `Windows` and `Unix` architectures. Both of them are marked as `final`. Testing: `jdk-tier1`, `jdk-tier2`, `jdk-tier3` and `java.naming` JCK tests show no failures with the change. - Commit messages: - 8283772: Make sun.net.dns.ResolverConfiguration sealed Changes: https://git.openjdk.java.net/jdk/pull/8006/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=8006&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8283772 Stats: 6 lines in 3 files changed: 0 ins; 0 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/8006.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8006/head:pull/8006 PR: https://git.openjdk.java.net/jdk/pull/8006
Integrated: 8282917: Remove InetAddressImplFactory from InetAddress
On Wed, 16 Mar 2022 13:26:36 GMT, Aleksei Efimov wrote: > Hi, > > This cleanup change removes `InetAddressImplFactory` class from > `InetAddress`. The list of changes: > - Remove `InetAddressImplFactory` from `InetAddress` > - Since `isIPv6Supported` native code is identical for Windows and Unix > implementations it was moved to the libnet's `InetAddress.c`. > - `InetAddressImplFactory.c` Windows and Unix implementations were removed > > Testing: jdk-tier1 to jdk-tier3 tests show no failures related to the change This pull request has now been integrated. Changeset: 929b6a35 Author:Aleksei Efimov URL: https://git.openjdk.java.net/jdk/commit/929b6a3556ce6f6ffb1a5ae14b7f87d21598eb21 Stats: 122 lines in 4 files changed: 15 ins; 105 del; 2 mod 8282917: Remove InetAddressImplFactory from InetAddress Reviewed-by: dfuchs, jpai - PR: https://git.openjdk.java.net/jdk/pull/7842
Re: RFR: 8282917: Remove InetAddressImplFactory from InetAddress [v4]
> Hi, > > This cleanup change removes `InetAddressImplFactory` class from > `InetAddress`. The list of changes: > - Remove `InetAddressImplFactory` from `InetAddress` > - Since `isIPv6Supported` native code is identical for Windows and Unix > implementations it was moved to the libnet's `InetAddress.c`. > - `InetAddressImplFactory.c` Windows and Unix implementations were removed > > Testing: jdk-tier1 to jdk-tier3 tests show no failures related to the change 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 five additional commits since the last revision: - Merge branch 'master' into JDK-8282917_Remove_InetAddressImplFactory - Update Java_java_net_InetAddress_isIPv4Available signature comment - Address review comments - Merge branch 'master' into JDK-8282917_Remove_InetAddressImplFactory - 8282917: Remove InetAddressImplFactory from InetAddress - Changes: - all: https://git.openjdk.java.net/jdk/pull/7842/files - new: https://git.openjdk.java.net/jdk/pull/7842/files/0b75bb45..77d0664f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7842&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7842&range=02-03 Stats: 7722 lines in 1062 files changed: 3843 ins; 1584 del; 2295 mod Patch: https://git.openjdk.java.net/jdk/pull/7842.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7842/head:pull/7842 PR: https://git.openjdk.java.net/jdk/pull/7842
Integrated: 8282536: java.net.InetAddress should be a sealed class
On Fri, 11 Mar 2022 16:47:46 GMT, Aleksei Efimov wrote: > The following fix seals the `java.net.InetAddress` class and permits only two > implementations - `java.net.Inet4Address` and `java.net.Inet6Address`. > > No issues have been detected by regression and JCK tests. > > Links: [JBS](https://bugs.openjdk.java.net/browse/JDK-8282536) > [CSR](https://bugs.openjdk.java.net/browse/JDK-8282880) This pull request has now been integrated. Changeset: 2b291d83 Author:Aleksei Efimov URL: https://git.openjdk.java.net/jdk/commit/2b291d837ebfd9d0a61f26541107c6a5f1d43773 Stats: 26 lines in 2 files changed: 1 ins; 21 del; 4 mod 8282536: java.net.InetAddress should be a sealed class Reviewed-by: dfuchs, jpai, rriggs, michaelm - PR: https://git.openjdk.java.net/jdk/pull/7789
Re: RFR: 8282917: Remove InetAddressImplFactory from InetAddress [v3]
On Wed, 16 Mar 2022 14:34:49 GMT, Daniel Fuchs wrote: >> I agree that it could be simplified to match >> `Java_java_net_InetAddress_isIPv4Available`. Changed in >> 49fdd576cade2e97639f827f9db6d0f1e31101e2 > > Thanks for noticing that Jaikiran! > > Both `ipv4_available()` and `ipv6_available()` are defined to return `jint` > so the implementation in `Java_java_net_InetAddress_isIPv6Supported` is > arguably the more correct (provided that the method being called returns 0 > when the corresponding IP version is not available). > > The signature in the comment should be changed as you suggest though. > > Possibly we should consider having both methods use the same construct... Current version should be ok since variables returned by `ipv4_available()` and `ipv6_available()` can only be initialized to `JNI_FALSE` or `JNI_TRUE`: - `Java_java_net_InetAddress_isIPv4Available`[InetAddress.c] returns result of `ipv4_available`[net_util.c] - `ipv4_available`[net_util.c] returns `IPv4_available` variable value - `IPv4_available` value is initalized with a result returned by `IPv4_supported()`[net_util_md.c] - `IPv4_supported()`[net_util_md.c] can return `JNI_TRUE` or `JNI_FALSE` only (in both Unix and Windows implementations) - Same stands true for `ipv6_available()`: only `JNI_TRUE` or `JNI_FALSE` can be returned - PR: https://git.openjdk.java.net/jdk/pull/7842
Re: RFR: 8282917: Remove InetAddressImplFactory from InetAddress [v3]
> Hi, > > This cleanup change removes `InetAddressImplFactory` class from > `InetAddress`. The list of changes: > - Remove `InetAddressImplFactory` from `InetAddress` > - Since `isIPv6Supported` native code is identical for Windows and Unix > implementations it was moved to the libnet's `InetAddress.c`. > - `InetAddressImplFactory.c` Windows and Unix implementations were removed > > Testing: jdk-tier1 to jdk-tier3 tests show no failures related to the change Aleksei Efimov has updated the pull request incrementally with one additional commit since the last revision: Update Java_java_net_InetAddress_isIPv4Available signature comment - Changes: - all: https://git.openjdk.java.net/jdk/pull/7842/files - new: https://git.openjdk.java.net/jdk/pull/7842/files/49fdd576..0b75bb45 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7842&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7842&range=01-02 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/7842.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7842/head:pull/7842 PR: https://git.openjdk.java.net/jdk/pull/7842
Re: RFR: 8282917: Remove InetAddressImplFactory from InetAddress [v2]
> Hi, > > This cleanup change removes `InetAddressImplFactory` class from > `InetAddress`. The list of changes: > - Remove `InetAddressImplFactory` from `InetAddress` > - Since `isIPv6Supported` native code is identical for Windows and Unix > implementations it was moved to the libnet's `InetAddress.c`. > - `InetAddressImplFactory.c` Windows and Unix implementations were removed > > Testing: jdk-tier1 to jdk-tier3 tests show no failures related to the change Aleksei Efimov has updated the pull request incrementally with one additional commit since the last revision: Address review comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/7842/files - new: https://git.openjdk.java.net/jdk/pull/7842/files/0af45a2d..49fdd576 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7842&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7842&range=00-01 Stats: 6 lines in 1 file changed: 0 ins; 4 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/7842.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7842/head:pull/7842 PR: https://git.openjdk.java.net/jdk/pull/7842
Re: RFR: 8282917: Remove InetAddressImplFactory from InetAddress [v2]
On Wed, 16 Mar 2022 13:49:39 GMT, Jaikiran Pai wrote:
>> Aleksei Efimov has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Address review comments
>
> src/java.base/share/native/libnet/InetAddress.c line 88:
>
>> 86: * Class: java_net_InetAddress
>> 87: * Method:isIPv6Supported
>> 88: * Signature: ()I
>
> Hello Aleksei, should this be `()Z` to represent the binary name of the
> boolean type
> https://docs.oracle.com/javase/specs/jvms/se17/html/jvms-4.html#jvms-4.3.2-200
Hi Jaikiran,
Yes, you're right - it should be `()Z`. The impl copied from
`src/java.base/unix/native/libnet/InetAddressImplFactory.c` had incorrect
header. Fixed in 49fdd576cade2e97639f827f9db6d0f1e31101e2
> src/java.base/share/native/libnet/InetAddress.c line 96:
>
>> 94: } else {
>> 95: return JNI_FALSE;
>> 96: }
>
> I don't have knowledge of C or JNI, but the
> `Java_java_net_InetAddress_isIPv4Available` currently does:
>
>
> return ipv4_available();
>
> So maybe a similar construct can be used here instead of if/else blocks? I'm
> guessing `ipv6_available()` will be returning values that are compatible with
> `JNI_TRUE/JNI_FALSE`.
I agree that it could be simplified to match
`Java_java_net_InetAddress_isIPv4Available`. Changed in
49fdd576cade2e97639f827f9db6d0f1e31101e2
-
PR: https://git.openjdk.java.net/jdk/pull/7842
RFR: 8282917: Remove InetAddressImplFactory from InetAddress
Hi, This cleanup change removes `InetAddressImplFactory` class from `InetAddress`. The list of changes: - Remove `InetAddressImplFactory` from `InetAddress` - Since `isIPv6Supported` native code is identical for Windows and Unix implementations it was moved to the libnet's `InetAddress.c`. - `InetAddressImplFactory.c` Windows and Unix implementations were removed Testing: jdk-tier1 to jdk-tier3 tests show no failures related to the change - Commit messages: - Merge branch 'master' into JDK-8282917_Remove_InetAddressImplFactory - 8282917: Remove InetAddressImplFactory from InetAddress Changes: https://git.openjdk.java.net/jdk/pull/7842/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=7842&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8282917 Stats: 125 lines in 4 files changed: 18 ins; 105 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/7842.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7842/head:pull/7842 PR: https://git.openjdk.java.net/jdk/pull/7842
Re: RFR: 8282536: java.net.InetAddress should be a sealed class [v3]
> The following fix seals the `java.net.InetAddress` class and permits only two > implementations - `java.net.Inet4Address` and `java.net.Inet6Address`. > > No issues have been detected by regression and JCK tests. > > Links: [JBS](https://bugs.openjdk.java.net/browse/JDK-8282536) > [CSR](https://bugs.openjdk.java.net/browse/JDK-8282880) 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 four additional commits since the last revision: - Merge branch 'master' into JDK-8282536_Seal_InetAddress - Cleanup unnessecary class loader checks, remove readObjectNoData - Add qualified Serializable usage with import - 8282536: java.net.InetAddress should be a sealed class - Changes: - all: https://git.openjdk.java.net/jdk/pull/7789/files - new: https://git.openjdk.java.net/jdk/pull/7789/files/f6901f09..44e6ab12 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7789&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7789&range=01-02 Stats: 10743 lines in 405 files changed: 6113 ins; 2397 del; 2233 mod Patch: https://git.openjdk.java.net/jdk/pull/7789.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7789/head:pull/7789 PR: https://git.openjdk.java.net/jdk/pull/7789
Re: RFR: 8282536: java.net.InetAddress should be a sealed class
On Fri, 11 Mar 2022 16:47:46 GMT, Aleksei Efimov wrote: > The following fix seals the `java.net.InetAddress` class and permits only two > implementations - `java.net.Inet4Address` and `java.net.Inet6Address`. > > No issues have been detected by regression and JCK tests. > > Links: [JBS](https://bugs.openjdk.java.net/browse/JDK-8282536) > [CSR](https://bugs.openjdk.java.net/browse/JDK-8282880) With only two sub-classes allowed the `InetAddress` code can be further cleaned-up by removing class loader checks. Pushed as f6901f098fe. Regression/JCK tests discovered no issues related to this change. - PR: https://git.openjdk.java.net/jdk/pull/7789
Re: RFR: 8282536: java.net.InetAddress should be a sealed class [v2]
> The following fix seals the `java.net.InetAddress` class and permits only two > implementations - `java.net.Inet4Address` and `java.net.Inet6Address`. > > No issues have been detected by regression and JCK tests. > > Links: [JBS](https://bugs.openjdk.java.net/browse/JDK-8282536) > [CSR](https://bugs.openjdk.java.net/browse/JDK-8282880) Aleksei Efimov has updated the pull request incrementally with one additional commit since the last revision: Cleanup unnessecary class loader checks, remove readObjectNoData - Changes: - all: https://git.openjdk.java.net/jdk/pull/7789/files - new: https://git.openjdk.java.net/jdk/pull/7789/files/2096e6b8..f6901f09 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7789&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7789&range=00-01 Stats: 23 lines in 2 files changed: 0 ins; 21 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/7789.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7789/head:pull/7789 PR: https://git.openjdk.java.net/jdk/pull/7789
RFR: 8282536: java.net.InetAddress should be a sealed class
The following fix seals the `java.net.InetAddress` class and permits only two implementations - `java.net.Inet4Address` and `java.net.Inet6Address`. No issues have been detected by regression and JCK tests. Links: [JBS](https://bugs.openjdk.java.net/browse/JDK-8282536) [CSR](https://bugs.openjdk.java.net/browse/JDK-8282880) - Commit messages: - Add qualified Serializable usage with import - 8282536: java.net.InetAddress should be a sealed class Changes: https://git.openjdk.java.net/jdk/pull/7789/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=7789&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8282536 Stats: 3 lines in 1 file changed: 1 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/7789.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7789/head:pull/7789 PR: https://git.openjdk.java.net/jdk/pull/7789
Re: RFR: 8279329: Remove hardcoded IPv4 available policy on Windows
On Wed, 2 Feb 2022 08:31:08 GMT, Masanori Yano wrote: > I added socket connection check same as IPv6_supported(). > Before this fix, when IPv4 is uninstalled (called `netsh interface ipv4 > uninstall`), and set property `-Djava.net.preferIPv4Stack=true`, > java.net.InetAddress.PLATFORM_LOOKUP_POLICY.characteristics is always set to > 1(IPv4), because IPv4_supported() always returns TRUE. After applied this > fix, java.net.InetAddress.PLATFORM_LOOKUP_POLICY.characteristics is set to > 2(IPv6). > It looks fine that IPv4_supported() returns FALSE. > Would you review this fix? The change looks good to me. - Marked as reviewed by aefimov (Committer). PR: https://git.openjdk.java.net/jdk/pull/7317
RFR: 8277628: Spec for InetAddressResolverProvider::get() throwing error or exception could be clearer
The following fix clarifies spec for `InetAddressResolverProvider.get(Configuration)` method to state that an error or exception thrown by this method will be propagated to to the caller of the method that triggered the lookup operation. The `InetAddressResolverProvider#system-wide-resolver` class-level javadoc section has been updated to state the same. - Commit messages: - 8277628: Spec for InetAddressResolverProvider::get() throwing error or exception could be clearer Changes: https://git.openjdk.java.net/jdk/pull/6725/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6725&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8277628 Stats: 3 lines in 1 file changed: 1 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/6725.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6725/head:pull/6725 PR: https://git.openjdk.java.net/jdk/pull/6725
Integrated: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI
On Tue, 5 Oct 2021 12:00:15 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 This pull request has now been integrated. Changeset: 2ca4ff87 Author:Aleksei Efimov URL: https://git.openjdk.java.net/jdk/commit/2ca4ff87b7c31d56542bbdcea70e828be33f4e97 Stats: 3141 lines in 56 files changed: 2848 ins; 155 del; 138 mod 8244202: Implementation of JEP 418: Internet-Address Resolution SPI Co-authored-by: Chris Hegarty Co-authored-by: Daniel Fuchs Co-authored-by: Alan Bateman Reviewed-by: dfuchs, alanb, michaelm, chegar - PR: https://git.openjdk.java.net/jdk/pull/5822
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v13]
> 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 pull request now contains 19 commits: - Merge branch 'master' into JDK-8244202_JEP418_impl - Merge branch 'master' into JDK-8244202_JEP418_impl - Merge branch 'master' into JDK-8244202_JEP418_impl - Replace 'system' with 'the system-wide', move comment section - Add @ throws NPE to hosts file resolver javadoc - 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) - ... and 9 more: https://git.openjdk.java.net/jdk/compare/f561d3c1...a2498d2b - Changes: https://git.openjdk.java.net/jdk/pull/5822/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5822&range=12 Stats: 3141 lines in 56 files changed: 2848 ins; 155 del; 138 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: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v12]
> 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 pull request now contains 18 commits: - Merge branch 'master' into JDK-8244202_JEP418_impl - Merge branch 'master' into JDK-8244202_JEP418_impl - Replace 'system' with 'the system-wide', move comment section - Add @ throws NPE to hosts file resolver javadoc - 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 - ... and 8 more: https://git.openjdk.java.net/jdk/compare/a316c06e...0542df51 - Changes: https://git.openjdk.java.net/jdk/pull/5822/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5822&range=11 Stats: 3141 lines in 56 files changed: 2848 ins; 155 del; 138 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: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v11]
> 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 17 additional commits since the last revision: - Merge branch 'master' into JDK-8244202_JEP418_impl - Replace 'system' with 'the system-wide', move comment section - Add @ throws NPE to hosts file resolver javadoc - 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 - ... and 7 more: https://git.openjdk.java.net/jdk/compare/9152e3fb...b31d61d1 - Changes: - all: https://git.openjdk.java.net/jdk/pull/5822/files - new: https://git.openjdk.java.net/jdk/pull/5822/files/f660cc6e..b31d61d1 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5822&range=10 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5822&range=09-10 Stats: 16073 lines in 487 files changed: 12211 ins; 1991 del; 1871 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: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v9]
On Wed, 27 Oct 2021 16:23:29 GMT, Michael McMahon wrote:
>> Aleksei Efimov has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Add @ throws NPE to hosts file resolver javadoc
>
> src/java.base/share/classes/java/net/InetAddress.java line 841:
>
>> 839: // 'resolver.lookupByAddress' and 'InetAddress.getAllByName0'
>> delegate to the system-wide resolver,
>> 840: // which could be a custom one. At that point we treat any
>> unexpected RuntimeException thrown by
>> 841: // the resolver as we would treat an UnknownHostException or an
>> unmatched host name.
>
> indentation issue in comment above
Thanks - moved the comment block inside `catch` block
(f660cc6ecc7a31c83de220160b9fd8d0fbacd1be)
> src/java.base/share/classes/java/net/InetAddress.java line 1308:
>
>> 1306: * Creates an InetAddress based on the provided host name and IP
>> address.
>> 1307: * System {@linkplain InetAddressResolver resolver} is not used to
>> check
>> 1308: * the validity of the address.
>
> Is this term "system resolver" defined somewhere? I think it means the
> configured resolver for the current VM. Previously, it really was the system
> resolver. So, I think it's potentially confusing, as there is also reference
> to the java.net.preferIPv6Addresses system property as having a possible
> value "system" which refers to the operating system. Since the CSR is
> approved, I'm happy to discuss this point post integration.
Thanks for highlighting that: Changed `"system"` to `"the system-wide"` -
that's what was originally meant by `"system resolver"`
(f660cc6ecc7a31c83de220160b9fd8d0fbacd1be).
-
PR: https://git.openjdk.java.net/jdk/pull/5822
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v10]
> 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: Replace 'system' with 'the system-wide', move comment section - Changes: - all: https://git.openjdk.java.net/jdk/pull/5822/files - new: https://git.openjdk.java.net/jdk/pull/5822/files/2ca78ba9..f660cc6e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5822&range=09 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5822&range=08-09 Stats: 8 lines in 1 file changed: 4 ins; 3 del; 1 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: 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&pr=5822&range=08 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5822&range=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: 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&pr=5822&range=07 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5822&range=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
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 [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 [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 [v5]
On Wed, 20 Oct 2021 15:41:35 GMT, Daniel Fuchs wrote: >> Aleksei Efimov has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Change InetAddressResolver method names > > Marked as reviewed by dfuchs (Reviewer). @dfuch @AlanBateman fa655be2bb0a402b70916567d34cc29a7898f938 and 2a554c91864e3b42a0ea315b0a671782fe341927 contain reworked `InetAddress`/`InetAddressResolverProvider`/`InetAddressResolver` specs with the following changes: - `InetAddress Resolver Providers` InetAddress section was modified and moved to `InetAddressResolverProvider`. - `Host Name Resolution` InetAddress section was updated to reference new InetAddress resolver SPI. - Changes for previous review comments. - javadoc formatting clean-up - PR: https://git.openjdk.java.net/jdk/pull/5822
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v7]
> 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: More javadoc updates to API classes - Changes: - all: https://git.openjdk.java.net/jdk/pull/5822/files - new: https://git.openjdk.java.net/jdk/pull/5822/files/2a554c91..fa655be2 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5822&range=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5822&range=05-06 Stats: 88 lines in 3 files changed: 22 ins; 8 del; 58 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: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v5]
On Wed, 20 Oct 2021 18:47:32 GMT, Alan Bateman wrote: >> Aleksei Efimov has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Change InetAddressResolver method names > > src/java.base/share/classes/java/net/InetAddress.java line 244: > >> 242: * @implNote >> 243: * For any lookup operation that might occur before the VM is fully >> booted the built-in >> 244: * resolver will be used. > > I think we will need decide if InetAddress class description is the right > place for this or whether some/most of it should move to > InetAddressResolverProvider. It might be that we update the existing "Host > Name Resolution" section with some basic/readable text to make the reader > aware that there is a provider mechanism with a link to > InetAddressResolverProvider with the details. Thanks for a good suggestion, Alan. It looks better now (2a554c91864e3b42a0ea315b0a671782fe341927) with some basic text for provider mechanism added to `Host Name Resolution`, and `InetAddress Resolver Providers` section moved to `InetAddressResolverProvider`. - PR: https://git.openjdk.java.net/jdk/pull/5822
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v6]
> 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: Review updates + move resolver docs to the provider class (CSR update to follow) - Changes: - all: https://git.openjdk.java.net/jdk/pull/5822/files - new: https://git.openjdk.java.net/jdk/pull/5822/files/30226481..2a554c91 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5822&range=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5822&range=04-05 Stats: 123 lines in 3 files changed: 49 ins; 40 del; 34 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: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v5]
> 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: Change InetAddressResolver method names - Changes: - all: https://git.openjdk.java.net/jdk/pull/5822/files - new: https://git.openjdk.java.net/jdk/pull/5822/files/ac0d2184..30226481 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5822&range=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5822&range=03-04 Stats: 33 lines in 9 files changed: 0 ins; 0 del; 33 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: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v3]
On Sun, 17 Oct 2021 21:03:56 GMT, Mark Sheppard wrote: > getByName requires a hostname lookup and getByAdress requires (eventually - I > know the docs says there’s no reverse lookup) an address reverse lookup. > Thus, a logical mapping is getByName —> lookupHostname, and getByAddr —> > lookupAddress, but the opposite will occur getByName --> lookupAddresses and > getByAddress --> lookupHostname. Therfore I proffer maps neatly to rename > methods to resolveHostname and resolveAddress such that the mapping are > getByName --> resolveHostname and getByAddress --> resolveAddress. Thank you, Mark, for highlighting the naming contradiction in the `InetAddressResolver` interface method names. It can be solved by making these names symmetrical to `InetAddress` API. In 302264810725f86a4569161754f7b052c78fd826 `InetAddressResolver` method names have been updated to stress input parameters type, which brings them in sync with `InetAddress` API: - `InetAddressResolver.lookupAddresses` is renamed to `InetAddressResolver.lookupByName` - `InetAddressResolver.lookupHostName` is renamed to `InetAddressResolver.lookupByAddress` - PR: https://git.openjdk.java.net/jdk/pull/5822
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v3]
On Sun, 17 Oct 2021 21:39:06 GMT, Mark Sheppard wrote: > I think that a hostname is constant while a host is up, but it can be > changed, and when changed a host restart is required. I don't think it is > quite as dynamic as has been suggested, but I open to correction. It is possible to change a host name without host restart. What has been tested: - Checked O/S type - Linux - `hostnamectl` cmd was used to change a host name - In output below there is only one `jshell` instance running. jshell> InetAddress.getLocalHost() $1 ==> test-host-name/192.168.0.155 $ hostnamectl set-hostname 'test-host-name-changed' # Need to sleep for 5 seconds since local host # is cached for 5 seconds since last resolution $ sleep 5 jshell> InetAddress.getLocalHost() $2 ==> test-host-name-changed/192.168.0.155 I believe it proves that we need to treat a host name as dynamically changing information. - PR: https://git.openjdk.java.net/jdk/pull/5822
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v3]
On Fri, 15 Oct 2021 17:19:26 GMT, Daniel Fuchs wrote:
>> Aleksei Efimov has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Add @since tags to new API classes
>> - Add checks and test for empty stream resolver results
>
> test/lib/jdk/test/lib/net/IPSupport.java line 88:
>
>> 86: } catch (SocketException se2) {
>> 87: return false;
>> 88: }
>
> The implementation of this method now conflicts with its name. Maybe we
> should pass a `Class` as parameter, and construct the
> loopback address from there. IIRC the issue with the original implementation
> was that it would say that IPv6 was not supported if IPv6 had only been
> disabled on the loopback interface - and that caused trouble because the
> native implementation of the built-in resolver had a different view of the
> situation - and saw that an IPv6 stack was available on the machine. Maybe
> this would deserve a comment too - so that future maintainers can figure out
> what we are trying to do here.
Thanks for a good suggestion: renamed to `isSupported`, changed parameter type
to `Class addressType` and updated it to use
`SocketChannel.open(ProtocolFamily pf)`
[API](https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/nio/channels/SocketChannel.html#open(java.net.ProtocolFamily))
added in `15` to check if the plafrom supports addresses of specified address
type. Pushed as 95a21e57fbe8be147d23e6a6901ae307e8237cbb.
All new tests (especially `LookupPolicyMappingTest`) pass in environment with
`IPv6` addresses disabled.
-
PR: https://git.openjdk.java.net/jdk/pull/5822
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v3]
On Fri, 15 Oct 2021 17:09:46 GMT, Daniel Fuchs wrote:
>> Aleksei Efimov has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Add @since tags to new API classes
>> - Add checks and test for empty stream resolver results
>
> test/jdk/java/net/spi/InetAddressResolverProvider/providers/recursive/recursive.init.provider/impl/InetAddressUsageInGetProviderImpl.java
> line 38:
>
>> 36: String localHostName;
>> 37: try {
>> 38: localHostName = InetAddress.getLocalHost().getHostName();
>
> Try to call InetAddress.getLocalHost().getHostName(); twice here.
New `BootstrapResolverUsageTest` test is added to trigger the bootstrap
resolver problem. `InetAddress.getByName` calls with unique names are used
instead of `InetAddress.getLocalHost()` to avoid caching of LHN resolution
results. Added in cd19ecd63838d3c2b713ed66d41107fe01a2d3e6.
-
PR: https://git.openjdk.java.net/jdk/pull/5822
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v3]
On Fri, 15 Oct 2021 14:25:02 GMT, Daniel Fuchs wrote:
>> Aleksei Efimov has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Add @since tags to new API classes
>> - Add checks and test for empty stream resolver results
>
> src/java.base/share/classes/java/net/InetAddress.java line 231:
>
>> 229: * The first provider found will be used to instantiate the {@link
>> InetAddressResolver InetAddressResolver}
>> 230: * by invoking the {@link
>> InetAddressResolverProvider#get(InetAddressResolverProvider.Configuration)}
>> 231: * method. The instantiated {@code InetAddressResolver} will be
>> installed as the system-wide
>
> Maybe "... method. The **returned** {@code InetAddressResolver} will be
> installed ..."
Changed as suggested in 648e65b8e6ae9687f7164ea3e81c51ab2b5e2dfe.
> src/java.base/share/classes/java/net/InetAddress.java line 486:
>
>> 484: return cns;
>> 485: } finally {
>> 486: bootstrapResolver = null;
>
> This is incorrect. This will set bootstrapResolver to null in the case where
> you return it at line 468 - which means that a second call will then find it
> null. I believe you want to set it to null in the finally clause only if you
> reached line 470. You could achieve this by declaring a local variable - e.g
> `InetAddressResolver tempResolver = null;` before entering the try-finally
> then set that variable at line 470 `return tempResolver = bootstrapResolver;`
> and in finally do `if (tempResolver == null) bootsrapResolver = null;`
>
> To test this you could try to call e.g. `InetAddress.getByName` twice in
> succession in your test: I believe it will fail with the code as it stands,
> but will succeed with my proposed changes...
Good catch, thank you Daniel. Added a test for that and fixed it in
cd19ecd63838d3c2b713ed66d41107fe01a2d3e6. Now the `bootstrapResolver` field is
set back to `null` in finally block only if a call to `InetAddress.resolver()`
entered a resolver initialization part.
> src/java.base/share/classes/java/net/InetAddress.java line 860:
>
>> 858: return host;
>> 859: }
>> 860: } catch (RuntimeException | UnknownHostException e) {
>
> This may deserve a comment: resolver.lookUpHostName and
> InetAddress.getAllbyName0 delegate to the system-wide resolver, which could
> be custom, and we treat any unexpected RuntimeException thrown by the
> provider at that point as we would treat an UnknownHostException or an
> unmatched host name.
Agree - comment added as part of 648e65b8e6ae9687f7164ea3e81c51ab2b5e2dfe.
> src/java.base/share/classes/java/net/InetAddress.java line 1063:
>
>> 1061: public Stream lookupAddresses(String host,
>> LookupPolicy policy)
>> 1062: throws UnknownHostException {
>> 1063: Objects.requireNonNull(host);
>
> Should we also double check that `policy` is not null? Or maybe assert it?
Yes, we should since custom resolvers might call the built-in one with `null` -
added non-null check in 648e65b .
> src/java.base/share/classes/java/net/InetAddress.java line 1203:
>
>> 1201: + " as hosts file " + hostsFile + " not found
>> ");
>> 1202: }
>> 1203: // Check number of found addresses:
>
> I wonder if the logic here could be simplified by first checking whether only
> IPv4 addresses are requested, and then if only IPv6 addresses are requested.
>
> That is - evacuate the simple cases first and then only deal with the more
> complex case where you have to combine the two lists...
Tried to simplify it in 648e65b8e6ae9687f7164ea3e81c51ab2b5e2dfe
-
PR: https://git.openjdk.java.net/jdk/pull/5822
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v4]
> 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 four additional commits since the last revision: - 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 - Changes: - all: https://git.openjdk.java.net/jdk/pull/5822/files - new: https://git.openjdk.java.net/jdk/pull/5822/files/d302a49a..ac0d2184 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5822&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5822&range=02-03 Stats: 245 lines in 5 files changed: 180 ins; 35 del; 30 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: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v3]
On Sat, 16 Oct 2021 10:48:32 GMT, Mark Sheppard wrote: > What’s in a name? I find the method names of the InetAddressResolver > interface a bit ambiguous. Typically in this DNS problem space one speaks of > lookup to resolve a hostname to its associated addresses and reverse lookup > to resolve an IP address to a hostname (or names) associated with it. I'm not sure that I see an ambiguity here: Interface has two methods, both are for lookup operations. That is why `lookup` word is used in both names. Then we put a description of a returned result after operation name: `Addresses` and `HostName` are the results. In cases when we want to highlight a parameter type in a method name we can use ‘by’/‘with’ prepositions: for instance, `InetAddress.getByName` `InetAddress.getByAddress` `ScheduledExecutorService.scheduleWithFixedDelay`. We can try and apply this approach to the description of DNS operations above: > lookup to resolve a hostname to its associated addresses operation: `lookup` result: `addresses` -> methodName: `lookupAddresses` > reverse lookup to resolve an IP address to a hostname operation: `lookup` (we've dropped reverse word here) result: `hostname` -> methodName: `lookupHostName` - PR: https://git.openjdk.java.net/jdk/pull/5822
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v3]
On Sat, 16 Oct 2021 12:30:44 GMT, Mark Sheppard wrote: > So Suggestion is refector remove Configuration to simplify the interface and > provide the BULITIN_RESOLVER and hostname as parameters to the > InetAddressResolverProvider::get method During work on this JEP we've examined the approach suggested above, and there are few issues with it: - `InetAddressResolverProvider.Configuration` interface API might evolve, e.g. there might be a need to pass additional configuration items. - local hostname is a dynamic information, therefore it cannot be passed as string parameter to `InetAddressResolverProvider::get`. It's been also discussed in the CSR comments [here](https://bugs.openjdk.java.net/browse/JDK-8274558) - PR: https://git.openjdk.java.net/jdk/pull/5822
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v3]
> 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 two additional commits since the last revision: - Add @since tags to new API classes - Add checks and test for empty stream resolver results - Changes: - all: https://git.openjdk.java.net/jdk/pull/5822/files - new: https://git.openjdk.java.net/jdk/pull/5822/files/41717fc7..d302a49a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5822&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5822&range=01-02 Stats: 150 lines in 6 files changed: 146 ins; 0 del; 4 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: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v2]
> 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 two additional commits since the last revision: - Merge branch 'master' into JDK-8244202_JEP418_impl - 8244202: Implementation of JEP 418: Internet-Address Resolution SPI - Changes: - all: https://git.openjdk.java.net/jdk/pull/5822/files - new: https://git.openjdk.java.net/jdk/pull/5822/files/77551538..41717fc7 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5822&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5822&range=00-01 Stats: 4216 lines in 179 files changed: 1929 ins; 1744 del; 543 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
RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI
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 - Commit messages: - 8244202: Implementation of JEP 418: Internet-Address Resolution SPI Changes: https://git.openjdk.java.net/jdk/pull/5822/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5822&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8244202 Stats: 2779 lines in 50 files changed: 2524 ins; 134 del; 121 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: 8274227: Remove "impl.prefix" jdk system property usage from InetAddress
On Wed, 29 Sep 2021 15:41:06 GMT, Aleksei Efimov wrote: > The following fix proposes to remove usages of `"impl.prefix"` JDK system > property from the `java.net.InetAddress` class. > This system property is used to locate concrete implementations of the > package private "java.net.InetAddressImpl" interface. > > The list of changes: > - `impl.prefix` usages are removed > - `InetAddressImpl` made sealed interface by only allowing default > implementations available in 'java.net' package: `Inet4AddressImpl` and > `Inet6AddressImpl`. > > tier1-tier3 tests show no failures with this fix. This pull request has now been integrated. Changeset: cc14c6f0 Author:Aleksei Efimov URL: https://git.openjdk.java.net/jdk/commit/cc14c6f076356731f78aea4e890027f4e2a91642 Stats: 53 lines in 4 files changed: 0 ins; 45 del; 8 mod 8274227: Remove "impl.prefix" jdk system property usage from InetAddress Reviewed-by: alanb, dfuchs - PR: https://git.openjdk.java.net/jdk/pull/5755
RFR: 8274227: Remove "impl.prefix" jdk system property usage from InetAddress
The following fix proposes to remove usages of `"impl.prefix"` JDK system property from the `java.net.InetAddress` class. This system property is used to locate concrete implementations of the package private "java.net.InetAddressImpl" interface. The list of changes: - `impl.prefix` usages are removed - `InetAddressImpl` made sealed interface by only allowing default implementations available in 'java.net' package: `Inet4AddressImpl` and `Inet6AddressImpl`. tier1-tier3 tests show no failures with this fix. - Commit messages: - Merge branch 'master' into JDK-8274227_remove_impl_prefix - 8274227: Seal java.net.InetAddressImpl - 8274227: Remove "impl.prefix" jdk system property usage from InetAddress Changes: https://git.openjdk.java.net/jdk/pull/5755/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5755&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8274227 Stats: 53 lines in 4 files changed: 0 ins; 45 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/5755.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5755/head:pull/5755 PR: https://git.openjdk.java.net/jdk/pull/5755
Integrated: 8273243: Fix indentations in java.net.InetAddress methods
On Wed, 1 Sep 2021 16:53:50 GMT, Aleksei Efimov wrote: > Hi, > > The fix changes indentations in the following `java.net.InetAddress` methods: > - getAddressesFromNameService > - getHostFromNameService > > It helps to improve code readability. Can't say the same about > `getHostFromNameService` diffs shown in this PR. This pull request has now been integrated. Changeset: 0c1b16b7 Author:Aleksei Efimov URL: https://git.openjdk.java.net/jdk/commit/0c1b16b75a2361431cbf9f4112dcd6049e981a78 Stats: 53 lines in 1 file changed: 9 ins; 11 del; 33 mod 8273243: Fix indentations in java.net.InetAddress methods Reviewed-by: dfuchs, bpb - PR: https://git.openjdk.java.net/jdk/pull/5336
Re: RFR: 8273243: Fix indentations in java.net.InetAddress methods [v2]
> Hi, > > The fix changes indentations in the following `java.net.InetAddress` methods: > - getAddressesFromNameService > - getHostFromNameService > > It helps to improve code readability. Can't say the same about > `getHostFromNameService` diffs shown in this PR. Aleksei Efimov has updated the pull request incrementally with one additional commit since the last revision: 8273243: looks -> looks like - Changes: - all: https://git.openjdk.java.net/jdk/pull/5336/files - new: https://git.openjdk.java.net/jdk/pull/5336/files/27c03f4d..f5627f73 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5336&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5336&range=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5336.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5336/head:pull/5336 PR: https://git.openjdk.java.net/jdk/pull/5336
Re: RFR: 8273243: Fix indentations in java.net.InetAddress methods [v2]
On Wed, 1 Sep 2021 17:08:24 GMT, Brian Burkhalter wrote: >> Aleksei Efimov has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8273243: looks -> looks like > > src/java.base/share/classes/java/net/InetAddress.java line 689: > >> 687: } >> 688: >> 689: //XXX: if it looks a spoof just return the address? > > looks _like_ a spoof Thanks Brian. Changed it as suggested :) - PR: https://git.openjdk.java.net/jdk/pull/5336
RFR: 8273243: Fix indentations in java.net.InetAddress methods
Hi, The fix changes indentations in the following `java.net.InetAddress` methods: - getAddressesFromNameService - getHostFromNameService It helps to improve code readability. Can't say the same about `getHostFromNameService` diffs shown in this PR. - Commit messages: - 8273243: Fix indentations in java.net.InetAddress methods Changes: https://git.openjdk.java.net/jdk/pull/5336/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5336&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8273243 Stats: 53 lines in 1 file changed: 9 ins; 11 del; 33 mod Patch: https://git.openjdk.java.net/jdk/pull/5336.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5336/head:pull/5336 PR: https://git.openjdk.java.net/jdk/pull/5336
Draft JEP: 8263693: InetAddress host name and address lookup service provider interface
Hi, For some time now we've been considering adding a standard service provider interface for InetAddress APIs that will allow deployment of alternative implementations of the underlying host name and address resolution mechanisms. After some prototyping work we have a draft JEP written: 8263693: InetAddress host name and address lookup service provider interface https://bugs.openjdk.java.net/browse/JDK-8263693 The prototype implementation (still work in progress) of this JEP can be viewed in JDK Sandbox branch: git clone https://github.com/openjdk/jdk-sandbox.git -b JDK-8244202-nspi-stream-branch As part of the prototyping work we've also developed two proof-of-concept name service providers to demonstrate and verify that the provider interface can be used to develop name service implementations alternative to the name service implementation shipped with the Java platform: PoC name service based on JNDI APIs: https://github.com/openjdk/jdk-sandbox/blob/JDK-8263693-nspi-pocs/JndiBasedPoC/src/main/java/jdk/test/nsp/proof/jndi/ProviderImpl.java PoC name service based on Netty library: https://github.com/openjdk/jdk-sandbox/blob/JDK-8263693-nspi-pocs/NettyBasedPoC/src/main/java/jdk/test/nsp/proof/netty/ProviderImpl.java Feedback on this JEP before submission would be very welcome. Best Regards, Aleksei
Re: RFR: 8264824: java/net/Inet6Address/B6206527.java doesn't close ServerSocket properly
On Mon, 12 Apr 2021 15:21:05 GMT, Conor Cleary wrote: > ### Description > `Inet6Address/B6206527.java` test creates two instances of ServerSocket, both > of which are explicity bound to a Link-Local address. Neither of the > ServerSocket instances are explicitly closed meaning there is no guarantee > that their associated resources are freed. > > ### Fix > Each ServerSocket is instantiated in a try-with-resources block. This ensures > that in both cases of success or failure within the try-with-resources block, > the sockets are always closed thanks to ServerSocket implementing Closeable. > The test is also now started in othervm mode as an added assurance of the > test's isolation in the event that resources are not freed. Hi Conor, The change looks good to me. - Marked as reviewed by aefimov (Committer). PR: https://git.openjdk.java.net/jdk/pull/3437
Integrated: 8264048: Fix caching in Jar URL connections when an entry is missing
On Tue, 30 Mar 2021 11:30:48 GMT, Aleksei Efimov wrote: > Current fix tries to tackle an issue with URL connection referencing > non-existing Jar file entries: > If an entry that doesn't exist is specified in an URL connection the > underlying Jar file is still cached even if an exception is thrown after > that. Such behavior prevents the caller, for instance, a `URLClassLoader`, > from closing a Jar file. > > The proposed fix checks if entry exists before caching a Jar file (only for > cases with enabled caching): > - If entry exists - jar file is cached if it wasn't cached before > - If entry doesn't exist and jar file wasn't cached before - jar file is > closed and exception is thrown > - If entry doesn't exist and jar file was cached before - jar file is kept > cached and exception is thrown > > > The following tests have been used to verify the fix: > - New regression tests > - ``:jdk_core:`` tests > - `api/java_util`,`api/java_net` JCK tests This pull request has now been integrated. Changeset: a611c462 Author:Aleksei Efimov URL: https://git.openjdk.java.net/jdk/commit/a611c462 Stats: 372 lines in 6 files changed: 346 ins; 2 del; 24 mod 8264048: Fix caching in Jar URL connections when an entry is missing Co-authored-by: Daniel Fuchs Reviewed-by: bchristi, dfuchs - PR: https://git.openjdk.java.net/jdk/pull/3263
Re: RFR: 8264048: Fix caching in Jar URL connections when an entry is missing [v2]
> Current fix tries to tackle an issue with URL connection referencing > non-existing Jar file entries: > If an entry that doesn't exist is specified in an URL connection the > underlying Jar file is still cached even if an exception is thrown after > that. Such behavior prevents the caller, for instance, a `URLClassLoader`, > from closing a Jar file. > > The proposed fix checks if entry exists before caching a Jar file (only for > cases with enabled caching): > - If entry exists - jar file is cached if it wasn't cached before > - If entry doesn't exist and jar file wasn't cached before - jar file is > closed and exception is thrown > - If entry doesn't exist and jar file was cached before - jar file is kept > cached and exception is thrown > > > The following tests have been used to verify the fix: > - New regression tests > - ``:jdk_core:`` tests > - `api/java_util`,`api/java_net` JCK tests Aleksei Efimov has updated the pull request incrementally with one additional commit since the last revision: JDK-8264048: Remove old version of RemoveJar test - Changes: - all: https://git.openjdk.java.net/jdk/pull/3263/files - new: https://git.openjdk.java.net/jdk/pull/3263/files/6aeb..2f0fa527 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3263&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3263&range=00-01 Stats: 179 lines in 1 file changed: 0 ins; 179 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/3263.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3263/head:pull/3263 PR: https://git.openjdk.java.net/jdk/pull/3263
Re: RFR: 8264048: Fix caching in Jar URL connections when an entry is missing
On Wed, 31 Mar 2021 11:24:02 GMT, Daniel Fuchs wrote: >> Current fix tries to tackle an issue with URL connection referencing >> non-existing Jar file entries: >> If an entry that doesn't exist is specified in an URL connection the >> underlying Jar file is still cached even if an exception is thrown after >> that. Such behavior prevents the caller, for instance, a `URLClassLoader`, >> from closing a Jar file. >> >> The proposed fix checks if entry exists before caching a Jar file (only for >> cases with enabled caching): >> - If entry exists - jar file is cached if it wasn't cached before >> - If entry doesn't exist and jar file wasn't cached before - jar file is >> closed and exception is thrown >> - If entry doesn't exist and jar file was cached before - jar file is kept >> cached and exception is thrown >> >> >> The following tests have been used to verify the fix: >> - New regression tests >> - ``:jdk_core:`` tests >> - `api/java_util`,`api/java_net` JCK tests > > Hi Aleksei, thanks for putting this together. > > `test/jdk/sun/misc/URLClassPath/RemoveJar.java` seems to be an older version > of `test/jdk/java/net/URLClassLoader/RemoveJar.java`. The two tests are > almost identical - so `test/jdk/sun/misc/URLClassPath/RemoveJar.java` can > probably be removed from the PR. > > Otherwise the proposed changes look good to me. Thanks for the review, Daniel. It is correct that `test/jdk/sun/misc/URLClassPath/RemoveJar.java` is an old version. It is removed now. - PR: https://git.openjdk.java.net/jdk/pull/3263
RFR: 8264048: Fix caching in Jar URL connections when an entry is missing
Current fix tries to tackle an issue with URL connection referencing non-existing Jar file entries: If an entry that doesn't exist is specified in an URL connection the underlying Jar file is still cached even if an exception is thrown after that. Such behavior prevents the caller, for instance, a `URLClassLoader`, from closing a Jar file. The proposed fix checks if entry exists before caching a Jar file (only for cases with enabled caching): - If entry exists - jar file is cached if it wasn't cached before - If entry doesn't exist and jar file wasn't cached before - jar file is closed and exception is thrown - If entry doesn't exist and jar file was cached before - jar file is kept cached and exception is thrown The following tests have been used to verify the fix: - New regression tests - ``:jdk_core:`` tests - `api/java_util`,`api/java_net` JCK tests - Commit messages: - Merge remote-tracking branch 'origin' into JDK-8264048 - 8264048: Fix caching in Jar URL connections when an entry is missing Changes: https://git.openjdk.java.net/jdk/pull/3263/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3263&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8264048 Stats: 551 lines in 7 files changed: 525 ins; 2 del; 24 mod Patch: https://git.openjdk.java.net/jdk/pull/3263.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3263/head:pull/3263 PR: https://git.openjdk.java.net/jdk/pull/3263
Re: RFR: 8262195: Harden tests that use the HostsFileNameService (jdk.net.hosts.file property) [v2]
On Thu, 25 Feb 2021 15:38:02 GMT, Conor Cleary wrote:
>> A number of net tests use a
>> **[HostsFileNameService](https://github.com/openjdk/jdk/blob/382e38dd246596ec94a1f1ce0e0f9e87f53366c7/src/java.base/share/classes/java/net/InetAddress.java#L955)**
>> to verify either the functionality of this type of Name Service or as a
>> complement to other tests (such as Caching, Address Format etc.).
>>
>> Intermittent failures of these tests can be caused by tools used during JVM
>> start-up accessing/initialising classes sooner than may be expected which
>> can cause unexpected behaviour. Most commonly, this unexpected behaviour
>> takes the form of the
>> **[PlatformNameService](https://github.com/openjdk/jdk/blob/382e38dd246596ec94a1f1ce0e0f9e87f53366c7/src/java.base/share/classes/java/net/InetAddress.java#L927)**
>> being used despite a call to `System.setProperty("jdk.net.hosts.file",
>> ...)` in the test file. Due to the fact that **InetAddress** initialises
>> it's Implementation and Name Service fields in a static class initialiser
>> ([see L1132 of
>> InetAddress.java](https://github.com/openjdk/jdk/blob/382e38dd246596ec94a1f1ce0e0f9e87f53366c7/src/java.base/share/classes/java/net/InetAddress.java#L1132))
>> and that the default mode is to use the **Platform Name Service**, setting
>> a system property in the main method to specify the use of
>> **HostsFileNameService** (with the jdk.net.hosts.file property)
has no affect if the class has been previously accessed during JVM start-up
and **InetAddress** will default to the **PlatformNameService** which is
unsuitable for this test. This explains the intermittent failures caused by the
use of `System.setProperty("jdk.net.hosts.file", ...)`.
>>
>> This fix improves the robustness of this test by specifying the use of the
>> **HostsFileNameService** in the `@run` tag for this test via the previously
>> mentioned system property. This gives certainty that the property will be
>> properly set in time for the actual run of this test after the JVM has
>> booted. An example of one the fixes...
>> * @run main/othervm -Djdk.net.hosts.file=TestToNumericFormatHosts
>> textToNumericFormat
>
> Conor Cleary has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - 8262195: Copyrights & InternalNSTest Host File Delete
> - 8262195: Added descriptive comments and filename
The latest changes look good to me. Thanks Conor.
-
Marked as reviewed by aefimov (Committer).
PR: https://git.openjdk.java.net/jdk/pull/2703
Re: RFR: 8262195: Harden tests that use the HostsFileNameService (jdk.net.hosts.file property)
On Wed, 24 Feb 2021 10:24:06 GMT, Conor Cleary wrote:
> A number of net tests use a
> **[HostsFileNameService](https://github.com/openjdk/jdk/blob/382e38dd246596ec94a1f1ce0e0f9e87f53366c7/src/java.base/share/classes/java/net/InetAddress.java#L955)**
> to verify either the functionality of this type of Name Service or as a
> complement to other tests (such as Caching, Address Format etc.).
>
> Intermittent failures of these tests can be caused by tools used during JVM
> start-up accessing/initialising classes sooner than may be expected which can
> cause unexpected behaviour. Most commonly, this unexpected behaviour takes
> the form of the
> **[PlatformNameService](https://github.com/openjdk/jdk/blob/382e38dd246596ec94a1f1ce0e0f9e87f53366c7/src/java.base/share/classes/java/net/InetAddress.java#L927)**
> being used despite a call to `System.setProperty("jdk.net.hosts.file", ...)`
> in the test file. Due to the fact that **InetAddress** initialises it's
> Implementation and Name Service fields in a static class initialiser ([see
> L1132 of
> InetAddress.java](https://github.com/openjdk/jdk/blob/382e38dd246596ec94a1f1ce0e0f9e87f53366c7/src/java.base/share/classes/java/net/InetAddress.java#L1132))
> and that the default mode is to use the **Platform Name Service**, setting a
> system property in the main method to specify the use of
> **HostsFileNameService** (with the jdk.net.hosts.file property)
has no affect if the class has been previously accessed during JVM start-up
and **InetAddress** will default to the **PlatformNameService** which is
unsuitable for this test. This explains the intermittent failures caused by the
use of `System.setProperty("jdk.net.hosts.file", ...)`.
>
> This fix improves the robustness of this test by specifying the use of the
> **HostsFileNameService** in the `@run` tag for this test via the previously
> mentioned system property. This gives certainty that the property will be
> properly set in time for the actual run of this test after the JVM has
> booted. An example of one the fixes...
> * @run main/othervm -Djdk.net.hosts.file=TestToNumericFormatHosts
> textToNumericFormat
Thanks for cleaning-up tests, Conor.
NIT: You also update/add the modification year in test headers.
test/jdk/java/net/InetAddress/InternalNameServiceTest.java line 30:
> 28: * a file name that contains address host mappings, similar to
> those in
> 29: * /etc/hosts file.
> 30: * @run main/othervm -Djdk.net.hosts.file=${test.src}/TestHosts
> -Dsun.net.inetaddr.ttl=0
Maybe the `TestHosts` file can be removed from the repository, and
`-Djdk.net.hosts.file` value can be changed to `TestHosts` file:
With such change the test host file will be generated in `scratch` directory,
and in case of test failures it won't modify the file stored in repository.
-
PR: https://git.openjdk.java.net/jdk/pull/2703
Integrated: 8259631: Reapply pattern match instanceof use in HttpClientImpl
On Tue, 12 Jan 2021 16:05:01 GMT, Aleksei Efimov wrote: > Hi, > > The proposed change adds back [1] `instanceof` pattern match use to > `HttpClientImpl` class. It was previously removed by > [JDK-8258696](https://bugs.openjdk.java.net/browse/JDK-8258696) due to docs > build failure. > > Aleksei > > [1] `git rollback be41468c --no-commit` This pull request has now been integrated. Changeset: b040a3d7 Author:Aleksei Efimov URL: https://git.openjdk.java.net/jdk/commit/b040a3d7 Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod 8259631: Reapply pattern match instanceof use in HttpClientImpl Reviewed-by: dfuchs, chegar - PR: https://git.openjdk.java.net/jdk/pull/2051
RFR: 8259631: Reapply pattern match instanceof use in HttpClientImpl
Hi, The proposed change adds back [1] `instanceof` pattern match use to `HttpClientImpl` class. It was previously removed by [JDK-8258696](https://bugs.openjdk.java.net/browse/JDK-8258696) due to docs build failure. Aleksei [1] `git rollback be41468c --no-commit` - Commit messages: - Revert "8258696: Temporarily revert use of pattern match instanceof until docs-reference is fixed" Changes: https://git.openjdk.java.net/jdk/pull/2051/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2051&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8259631 Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2051.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2051/head:pull/2051 PR: https://git.openjdk.java.net/jdk/pull/2051
Re: RFR: 8258422: Cleanup unnecessary null comparison before instanceof check in java.base
On Mon, 11 Jan 2021 23:29:53 GMT, Aleksei Efimov wrote: >> @AlekseiEfimov `HttpClientImpl` is not in `java.base` module. So I think >> it's better to not touch it under this PR. > > To double check that docs build will be stable after integration the > following actions have been performed: > - The pull request branch was locally synced-up with the latest changes from > `master` (`JDK-8258657` is part of them) > - Verified that local `docs-reference-api-javadoc` build completes > successfully sponsor - PR: https://git.openjdk.java.net/jdk/pull/20
Re: RFR: 8258422: Cleanup unnecessary null comparison before instanceof check in java.base
On Mon, 11 Jan 2021 17:12:24 GMT, Andrey Turbanov wrote: >> Hi @turbanoff, >> This PR is ready for integration - `JDK-8258657` has been resolved. >> You can proceed with issuing `integrate` bot command. Then I will `sponsor` >> it. >> But before that, I would like to ask if you would like to take on board the >> changes to `HttpClientImpl`? Alternatively, I can follow it up separately >> and reapply the backed-out change under different bugID. > > @AlekseiEfimov `HttpClientImpl` is not in `java.base` module. So I think it's > better to not touch it under this PR. To double check that docs build will be stable after integration the following actions have been performed: - The pull request branch was locally synced-up with the latest changes from `master` (`JDK-8258657` is part of them) - Verified that local `docs-reference-api-javadoc` build completes successfully - PR: https://git.openjdk.java.net/jdk/pull/20
Re: RFR: 8258422: Cleanup unnecessary null comparison before instanceof check in java.base
On Sat, 5 Sep 2020 18:55:53 GMT, Andrey Turbanov wrote: > 8258422: Cleanup unnecessary null comparison before instanceof check in > java.base Hi @turbanoff, This PR is ready for integration - `JDK-8258657` has been resolved. You can proceed with issuing `integrate` bot command. Then I will `sponsor` it. But before that, I would like to ask if you would like to take on board the changes to `HttpClientImpl`? Alternatively, I can follow it up separately and reapply the backed-out change under different bugID. - PR: https://git.openjdk.java.net/jdk/pull/20
Re: RFR: 8258422: Cleanup unnecessary null comparison before instanceof check in java.base [v4]
On Sat, 19 Dec 2020 10:29:23 GMT, Chris Hegarty wrote: >> Thank you for checking `HttpURLConnection` and `Socket`. >> The latest version looks good to me. > > This issue is blocked by [8258657][1]. It should not be integrated until > after 8258657 has been resolved. The JIRA issues have been linked up to > reflect this. > > [1]: https://bugs.openjdk.java.net/browse/JDK-8258657 [JDK-8258657](https://bugs.openjdk.java.net/browse/JDK-8258657) has been resolved. The changes reverted by [JDK-8258696](https://bugs.openjdk.java.net/browse/JDK-8258696) could also be re-applied to `HttpClientImpl` class. - PR: https://git.openjdk.java.net/jdk/pull/20
Re: RFR: 8258696: Temporarily revert use of pattern match instanceof until docs-reference is fixed
On Fri, 18 Dec 2020 16:54:59 GMT, Chris Hegarty wrote:
> Temporarily revert use of pattern match instanceof construct until
> docs-reference is fixed, see JDK-8258657.
>
> ...
> Generating REFERENCE_API javadoc for 21 modules
>
> if (delegate instanceof ExecutorService service) {
> ^
> (use --enable-preview to enable pattern matching in instanceof)
> 1 error
> make[3]: ***
> [/Users/chhegar/git/open/build/macosx-x64/support/docs/_javadoc_REFERENCE_API_exec.marker]
> Error 1
> Docs.gmk:472: recipe for target
> '/Users/chhegar/git/open/build/macosx-x64/support/docs/_javadoc_REFERENCE_API_exec.marker'
> failed
> make/Main.gmk:485: recipe for target 'docs-reference-api-javadoc' failed
> make[2]: *** [docs-reference-api-javadoc] Error 2
> make[2]: *** Waiting for unfinished jobs
Marked as reviewed by aefimov (Committer).
-
PR: https://git.openjdk.java.net/jdk/pull/1843
Re: RFR: 8258422: Cleanup unnecessary null comparison before instanceof check in java.base [v4]
On Wed, 16 Dec 2020 10:32:12 GMT, Andrey Turbanov wrote: >> 8258422: Cleanup unnecessary null comparison before instanceof check in >> java.base > > Andrey Turbanov has updated the pull request incrementally with one > additional commit since the last revision: > > 8258422: Cleanup unnecessary null comparison before instanceof check in > java.base > take advantage of "flow scoping" to eliminate casts Thank you for checking `HttpURLConnection` and `Socket`. The latest version looks good to me. - Marked as reviewed by aefimov (Committer). PR: https://git.openjdk.java.net/jdk/pull/20
Re: RFR: 8258422: Cleanup unnecessary null comparison before instanceof check in java.base [v3]
On Wed, 16 Dec 2020 09:44:37 GMT, Chris Hegarty wrote:
>> Andrey Turbanov has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8258422: Cleanup unnecessary null comparison before instanceof check in
>> java.base
>> use instanceof pattern matching in UnixPath too
>
> Let's take advantage of "flow scoping" to eliminate some of these casts. A
> few examples follow.
Hi Andrey,
Could you, please, also take a look at `java.net.Socket`:
java/net/Socket.java:if (bindpoint != null && (!(bindpoint instanceof
InetSocketAddress)))
java/net/Socket.java-throw new
IllegalArgumentException("Unsupported address type");
And `HttpURLConnection`:
sun/net/www/protocol/http/HttpURLConnection.java:if (a
!= null && c instanceof HttpURLConnection) {
sun/net/www/protocol/http/HttpURLConnection.java-
((HttpURLConnection)c).setAuthenticator(a);
The following cmd was used to find them: `rgrep -A 1 "= null .* instanceof "`
-
PR: https://git.openjdk.java.net/jdk/pull/20
Re: RFR: 8258422: Cleanup unnecessary null comparison before instanceof check in java.base
On Wed, 2 Dec 2020 20:15:02 GMT, Andrey Turbanov
wrote:
>> This seems like a reasonable change, which improves readability.
>>
>> My preference is to wait a little longer (hopefully no more than a couple of
>> weeks), until [JEP 394](https://openjdk.java.net/jeps/394) - "Pattern
>> Matching for instanceof" is finalised, then we can remove the explicit casts
>> in many of these cases. For example:
>>
>> --- a/src/java.base/share/classes/java/io/File.java
>> +++ b/src/java.base/share/classes/java/io/File.java
>> @@ -2191,8 +2191,8 @@ public class File
>> * {@code false} otherwise
>> */
>> public boolean equals(Object obj) {
>> -if ((obj != null) && (obj instanceof File)) {
>> -return compareTo((File)obj) == 0;
>> +if (obj instanceof File file) {
>> +return compareTo(file) == 0;
>> }
>> return false;
>> }
>
> Related issue - https://bugs.openjdk.java.net/browse/JDK-8257448
Hi @turbanoff,
I've logged a JBS issue for tracking this change:
https://bugs.openjdk.java.net/browse/JDK-8258422
JEP 394 is finalized now, so I guess you could follow-up Chris suggestion to
remove the explicit casts.
After the fix is properly reviewed and marked as ready for the integration
(you'll need to issue `/integrate` command), once it is done I would happily
`/sponsor` the change.
With Best Regards,
Aleksei
-
PR: https://git.openjdk.java.net/jdk/pull/20
Re: RFR: 8236413: AbstractConnectTimeout should tolerate both NoRouteToHostException and UnresolvedAddressException
On Fri, 4 Dec 2020 17:41:47 GMT, Daniel Fuchs wrote: > The timeout tests extending AbstractConnectTimeout have been relaxed to > accept a > ConnectException wrapping NoRouteToHostException. > These test have been observed failing intermittently with ConnectException > wrapping > java.nio.channels.UnresolvedAddressException instead - presumably a transient > network > failure caused this other exception to be thrown. > > The test logic should be relaxed to accept UnresolvedAddressException as > possible > cause in ConnectException too. Hi Daniel, The changes look good to me 👍 - Marked as reviewed by aefimov (Committer). PR: https://git.openjdk.java.net/jdk/pull/1629
