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 [v3]
On Sun, 17 Oct 2021 15:55:59 GMT, Aleksei Efimov 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) OK grand ... I didn't see the CSR discussion :-( WRT hostname being dynamic and requires a lambda ... is it really dynamic? I know it can be changed, but IIRC most (unix and windows) if not all systems require a restart for such a change change to take effect, and usually requires a dns cache flush also, in which case an application will be restarting also. In the case of working over VPN, VPN software usually changes the hostname dynamically, but that name change is only valid for the duration of the VPN session, and when you exit the VPN, it reverts back to the original hostname. In such cases an application will need to be restarted. There's no seemless working from VPN to non VPN. 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 something that can be investigated. - 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 16:33:57 GMT, Aleksei Efimov 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` Yes, ambiguity wasn’t quite the word I was looking for. Maybe contradictory or oxymoron might do !! Let me try and clarify the rational behind the suggestion. To re-iterate, the parlance of the DNS domain in context of address resolution is lookup for hostname to IP address resolution and reverse lookup for address to hostname mapping. For me, the natural verb to method translation is lookup and **reverseLookup OR resolveHostname and resolveAddress. The latter names are pretty definitive and unambiguous. It clear and obvious without the need to pursue the full signature of the method, what the semantics of the methods should be. At the java InetAddress API level there is getByName(String host) and getByAddress(byte[] addr). At the native OS level there is gethostbyname, gethostbyaddr and the evolved api getnameinfo and getaddrinfo. 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. I will concede that it can be reasonable argued that the native library functions getnameinfo and getaddrinfo may better abstract to lookupHostname and lookupAddresses respectively. But, I have always found these two library calls a bit contorted and the legacy gethostbyname and gethostbyaddress were more logical. In you proposition above: > operation: `lookup` result: `addresses` -> methodName: `lookupAddresses` > operation: `lookup` (we've dropped reverse word here) result: `hostname` -> > methodName: `lookupHostName` You are generating the name from the result of the action. I’m arguing that it should be based on the operation on the subject i.e. the parameter being passed. If this was hashmap lookup, then it is an operation of mapping hostname to addresses mapHostnameToAddresses, i.e. looking up a hostname, and mapping an address to a hostname mapAddressToHostname i.e. looking up an address. Of course, it is possible to just **overload** a lookup method, one taking Sting and t’other taking byte adder in the method’s parameters, respectively. Then, you don’t have to concede too much in this argument. But, for me as an ordinary developer, resolveAddress and resolveHostname are crystal clear and unambiguous. - 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 15:55:59 GMT, Aleksei Efimov wrote: > * `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`. Right, I think we've ended up with a good model here that supports the common case, stacking, and can be extended in the future to add new configuration if required. Also if good use-cases come up when Configuration can be changed to be non-sealed. - 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]
On Tue, 12 Oct 2021 15:43:24 GMT, Aleksei Efimov wrote: >> This change implements a new service provider interface for host name and >> address resolution, so that java.net.InetAddress API can make use of >> resolvers other than the platform's built-in resolver. >> >> The following API classes are added to `java.net.spi` package to facilitate >> this: >> - `InetAddressResolverProvider` - abstract class defining a service, and >> is, essentially, a factory for `InetAddressResolver` resolvers. >> - `InetAddressResolverProvider.Configuration ` - an interface describing the >> platform's built-in configuration for resolution operations that could be >> used to bootstrap a resolver construction, or to implement partial >> delegation of lookup operations. >> - `InetAddressResolver` - an interface that defines methods for the >> fundamental forward and reverse lookup operations. >> - `InetAddressResolver.LookupPolicy` - a class whose instances describe the >> characteristics of one forward lookup operation. >> >> More details in [JEP-418](https://openjdk.java.net/jeps/418). >> >> Testing: new and existing `tier1:tier3` tests > > Aleksei Efimov has updated the pull request incrementally with two additional > commits since the last revision: > > - Add @since tags to new API classes > - Add checks and test for empty stream resolver results Refactor remove Configuration and simplify interface: In the InetAddressResolver a Configuration abstraction is defined, and this is supplied to a get method of the InetAddressResolverProvider. The objective is to “inject” some platform configuration into an InetAddressResolver. This consistents of two pieces of platform configuration detail, a builtin resolver reference and the hostname. Why not provide them as parameters to the provider get method and dispense with the Configuration abstraction? Thus simplifying the interface. The hostname is being supplied by the getLocalHostName of an InetAddressImpl. This is essentially a wrapper to the library call gethostname. This could be provided as a static method in InetAddressImpl, called once during loadResolver to provide the hostname parameter String. The hostname is a static piece of system configuration, which requires a system restart if it were to be changed - so need to provide a lambda. So Suggestion is refector remove Configuration to simplify the interface and provide the BULITIN_RESOLVER and hostname as parameters to the InetAddressResolverProvider::get method - PR: https://git.openjdk.java.net/jdk/pull/5822
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v3]
On Tue, 12 Oct 2021 15:43:24 GMT, Aleksei Efimov wrote: >> This change implements a new service provider interface for host name and >> address resolution, so that java.net.InetAddress API can make use of >> resolvers other than the platform's built-in resolver. >> >> The following API classes are added to `java.net.spi` package to facilitate >> this: >> - `InetAddressResolverProvider` - abstract class defining a service, and >> is, essentially, a factory for `InetAddressResolver` resolvers. >> - `InetAddressResolverProvider.Configuration ` - an interface describing the >> platform's built-in configuration for resolution operations that could be >> used to bootstrap a resolver construction, or to implement partial >> delegation of lookup operations. >> - `InetAddressResolver` - an interface that defines methods for the >> fundamental forward and reverse lookup operations. >> - `InetAddressResolver.LookupPolicy` - a class whose instances describe the >> characteristics of one forward lookup operation. >> >> More details in [JEP-418](https://openjdk.java.net/jeps/418). >> >> Testing: new and existing `tier1:tier3` tests > > Aleksei Efimov has updated the pull request incrementally with two additional > commits since the last revision: > > - Add @since tags to new API classes > - Add checks and test for empty stream resolver results 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. For the method names lookupHostname, and lookupAddresses, and the semantics conveyed in those names, I would expect lookupHostname to resolve a hostname, that is, take a hostname as parameter and the return value the addresses, while lookupAddresses I would expect to resolve an address to its hostname, that is, take an address and return a hostname. However, the current pair of methods names convey the opposite, lookupHostname resolves an address and lookupAddresses resolves a hostname. I would proffer a method name change to use lookup and reverseLookup OR to use resolvesHostname and resolveAddress, to resolve a hostname and address respectively. Thus, lookupHostname should be renamed to reverseLookup and lookupAddresses to lookup OR lookupHostname renamed to resolveAddress and lookupAddresses renamed to resolveHostname. - PR: https://git.openjdk.java.net/jdk/pull/5822
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v3]
On Tue, 12 Oct 2021 15:43:24 GMT, Aleksei Efimov wrote:
>> This change implements a new service provider interface for host name and
>> address resolution, so that java.net.InetAddress API can make use of
>> resolvers other than the platform's built-in resolver.
>>
>> The following API classes are added to `java.net.spi` package to facilitate
>> this:
>> - `InetAddressResolverProvider` - abstract class defining a service, and
>> is, essentially, a factory for `InetAddressResolver` resolvers.
>> - `InetAddressResolverProvider.Configuration ` - an interface describing the
>> platform's built-in configuration for resolution operations that could be
>> used to bootstrap a resolver construction, or to implement partial
>> delegation of lookup operations.
>> - `InetAddressResolver` - an interface that defines methods for the
>> fundamental forward and reverse lookup operations.
>> - `InetAddressResolver.LookupPolicy` - a class whose instances describe the
>> characteristics of one forward lookup operation.
>>
>> More details in [JEP-418](https://openjdk.java.net/jeps/418).
>>
>> Testing: new and existing `tier1:tier3` tests
>
> Aleksei Efimov has updated the pull request incrementally with 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.
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.
-
PR: https://git.openjdk.java.net/jdk/pull/5822
Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v3]
On Tue, 12 Oct 2021 15:43:24 GMT, Aleksei Efimov wrote:
>> This change implements a new service provider interface for host name and
>> address resolution, so that java.net.InetAddress API can make use of
>> resolvers other than the platform's built-in resolver.
>>
>> The following API classes are added to `java.net.spi` package to facilitate
>> this:
>> - `InetAddressResolverProvider` - abstract class defining a service, and
>> is, essentially, a factory for `InetAddressResolver` resolvers.
>> - `InetAddressResolverProvider.Configuration ` - an interface describing the
>> platform's built-in configuration for resolution operations that could be
>> used to bootstrap a resolver construction, or to implement partial
>> delegation of lookup operations.
>> - `InetAddressResolver` - an interface that defines methods for the
>> fundamental forward and reverse lookup operations.
>> - `InetAddressResolver.LookupPolicy` - a class whose instances describe the
>> characteristics of one forward lookup operation.
>>
>> More details in [JEP-418](https://openjdk.java.net/jeps/418).
>>
>> Testing: new and existing `tier1:tier3` tests
>
> Aleksei Efimov has updated the pull request incrementally with 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 ..."
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...
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.
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?
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...
-
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=5822=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5822=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
