Re: RFR: 8275640 (win) java.net.NetworkInterface issues with IPv6-only environments [v7]

[Mark Sheppard]

On Mon, 7 Feb 2022 09:14:51 GMT, Daniel Jeliński  wrote:

>> Clean up of various issues related to error handling and memory management
>
> Daniel Jeliński has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Initialize return value in all cases

Marked as reviewed by msheppar (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/6090

Re: RFR: 8279329: Remove hardcoded IPv4 available policy on Windows

[Mark Sheppard]

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?

I was working on this -- we have yet to fully analyse various failures from a 
Windows IPv6 only run, the build for which also  has changes from 
https://bugs.openjdk.java.net/browse/JDK-8279566 and some changes from 
https://bugs.openjdk.java.net/browse/JDK-8275640
We should have access to the test environment again next week to  execute 
additional test runs

-

PR: https://git.openjdk.java.net/jdk/pull/7317

Re: RFR: 8278961: Enable debug logging in java/net/DatagramSocket/SendDatagramToBadAddress.java

[Mark Sheppard]

On Wed, 12 Jan 2022 13:36:19 GMT, Jaikiran Pai  wrote:

>> as I indicated below, we have tracked and investigated this issue and it is 
>> not purely a macosx-aarch64 issue. It may also be a  test env issue. As such 
>> using the OS filtering in the test, which is designed to primarily exclued 
>> AIX, wouldn't be the appropraite approach. If we wish to exclude the test on 
>> macosx-aarch64, then use of Problemlist is more appropriate.
>
> Hello Mark,
> 
> I just noticed this comment. My understanding of the comment is that you 
> don't expect any other changes to this test other than what has already been 
> done in this PR to enable the debug logs? Is that correct?

yes, the addition of the diagnostics is useful

-

PR: https://git.openjdk.java.net/jdk/pull/6883

Re: RFR: 8278961: Enable debug logging in java/net/DatagramSocket/SendDatagramToBadAddress.java

[Mark Sheppard]

On Fri, 17 Dec 2021 14:52:53 GMT, Jaikiran Pai  wrote:

> Can I please get a review for this test only change which proposes to enable 
> debug logs from the test that failed intermittently? This change addresses 
> https://bugs.openjdk.java.net/browse/JDK-8278961.
> 
> The change passes the (test specific) `-d` option to enable logs from that 
> test by default. While I was at it, I even added a few more debug logs hoping 
> it might provide some hints if/when it fails next.
> 
> For reference, a (successful) run of this test will now print something like:
> 
> 
> --System.out:(18/930)--
> running on OS that supports ICMP port unreachable
> Testing with class java.net.DatagramSocket
> tests will be run against destination address localhost/127.0.0.1 port 52682
> Checking send to connected address ...
> socket is locally bound to address /127.0.0.1 port 52681
> received data from address /127.0.0.1 port 52681
> received data from address /127.0.0.1 port 52681
> received data from address /127.0.0.1 port 52681
> received data from address /127.0.0.1 port 52681
> received data from address /127.0.0.1 port 52681
> Checking send to non-connected address ...
> received data from address /127.0.0.1 port 52681
> Checking send to invalid address ...
> Got expected exception: java.net.PortUnreachableException
> Got expected exception: java.net.PortUnreachableException
> Got expected exception: java.net.PortUnreachableException
> Got expected exception: java.net.PortUnreachableException
> Got expected exception: java.net.PortUnreachableException

Marked as reviewed by msheppar (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/6883

Re: RFR: 8275640 (win) java.net.NetworkInterface issues with IPv6-only environments [v4]

[Mark Sheppard]

On Wed, 12 Jan 2022 08:09:59 GMT, Daniel Jelinski  wrote:

>> Clean up of various issues related to error handling and memory management
>
> Daniel Jelinski has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Remove unused / incorrect exit code -2 from enumInterfaces

src/java.base/windows/native/libnet/NetworkInterface.c line 216:

> 214: break;
> 215: }
> 216: return -1;

*netifPP = NULL;

and a similar NULL out value for all  return -1  in this function

-

PR: https://git.openjdk.java.net/jdk/pull/6090

Re: RFR: 8275640 (win) java.net.NetworkInterface issues with IPv6-only environments [v3]

[Mark Sheppard]

On Mon, 10 Jan 2022 07:37:24 GMT, Daniel Jelinski  wrote:

>> src/java.base/windows/native/libnet/NetworkInterface_winXP.c line 256:
>> 
>>> 254: 
>>> 255: ret = enumInterfaces(env, netifPP);
>>> 256: if (ret == -1) {
>> 
>> this change is questionable: enumInterfaces returns -2  to allows processing 
>> to continue with IPv6 data ... this change will prohibit that policy
>
> In my experiments the `enumInterfaces` succeeded in IPv6-only environment. 
> The function only fails with -2 when a new interface is added during 
> enumeration.
> I could modify the function to stop returning -2 if you think it makes sense.

yes, that's what I have onserved also It looks like the error handling in 
enumInterfaces is cut and paste from lookupIPAddrTable, as per the comment

// this different error code is to handle the case when we call
// GetIpAddrTable in pure IPv6 environment
return -2;
when in fact it is calling GetIfTable

and if that returns an error, it seems reasonable to terminate processing at 
that stage by throwing an appropriate exception ... I can't see how it can 
continue

So it would seems that enumInterfaces curennt return value -2 is misleading

-

PR: https://git.openjdk.java.net/jdk/pull/6090

Re: RFR: 8275640 (win) java.net.NetworkInterface issues with IPv6-only environments [v3]

[Mark Sheppard]

On Sat, 8 Jan 2022 09:11:05 GMT, Daniel Jelinski  wrote:

>> Clean up of various issues related to error handling and memory management
>
> Daniel Jelinski has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Address problems reported by clang-tidy

src/java.base/windows/native/libnet/NetworkInterface_winXP.c line 282:

> 280: }
> 281: while (curr != NULL) {
> 282: netaddr *netaddrP;

netaddr *netaddrP = NULL;  just in case enumAddresses_win_ipaddrtable does not 
update it for an error return

-

PR: https://git.openjdk.java.net/jdk/pull/6090

Re: RFR: 8275640 (win) java.net.NetworkInterface issues with IPv6-only environments [v3]

[Mark Sheppard]

On Sat, 8 Jan 2022 09:11:05 GMT, Daniel Jelinski  wrote:

>> Clean up of various issues related to error handling and memory management
>
> Daniel Jelinski has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Address problems reported by clang-tidy

I'm working through a number of related issues in this area  I have applied 
your changes and run InetAddress and NetworkInterface sanity checks, and 
encountered some problems with these changes - as such we're working through 
the failures analysis - will provide approriate feedback in due course

-

PR: https://git.openjdk.java.net/jdk/pull/6090

Re: RFR: 8275640 (win) java.net.NetworkInterface issues with IPv6-only environments [v3]

[Mark Sheppard]

On Sat, 8 Jan 2022 09:11:05 GMT, Daniel Jelinski  wrote:

>> Clean up of various issues related to error handling and memory management
>
> Daniel Jelinski has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Address problems reported by clang-tidy

src/java.base/windows/native/libnet/NetworkInterface_winXP.c line 271:

> 269: curr = *netifPP;
> 270: ret = lookupIPAddrTable(env, );
> 271: if (ret == -1) {

on the face of it, this change looks reasonable as one might think "let the 
processing continue for IPv6", however it introduces some problems in 
subsequent processing. Most notably, it assigns a NULL value to tableP , which 
is then fed to the function enumAddresses_win_ipaddrtable and this performs a 
sanity check for tableP == NULL and returns 0. Also 
enumAddresses_win_ipaddrtable takes the inout parameter netaddr *netaddrP; 
which is a local variable (stack variable) which hasn't been properly 
initialized and only has a valid value assigned for a successful return from 
enumAddresses_win_ipaddrtable. Thus, this inout varaiable has indeterminates 
values (i.e. what ever is on the stack) which are then assigned in the else 
block (for the return 0 case)
 else{
curr->addrs = netaddrP;
curr->naddrs += ret;
curr = curr->next;
}
and so subsequent indeterminate processing and potental crash is significant as 
the netif data structures spurious pointer assignments

I think the current coded return policy is good, but should probably include 
*netifPP = NULL as nothing is being returned.

The function getAllInterfacesAndAddresses also inconsistently handles the out 
value of *netifPP as it is not always assigned a value in 
enumAddresses_win_ipaddrtable. As such the addition of call to free_netif  on 
this pointer may lead to indeterminate behavious -- Thus the assignment and 
update of netifPP needs to be addressed

src/java.base/windows/native/libnet/NetworkInterface_winXP.c line 285:

> 283: ret = enumAddresses_win_ipaddrtable(env, curr, , 
> tableP);
> 284: if (ret < 0) {
> 285: free_netif(*netifPP);

seems like the right thing to do, but *netifPP may not be properly assigned a 
valid pointer

-

PR: https://git.openjdk.java.net/jdk/pull/6090

Re: RFR: 8275640 (win) java.net.NetworkInterface issues with IPv6-only environments [v3]

[Mark Sheppard]

On Sat, 8 Jan 2022 09:11:05 GMT, Daniel Jelinski  wrote:

>> Clean up of various issues related to error handling and memory management
>
> Daniel Jelinski has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Address problems reported by clang-tidy

src/java.base/windows/native/libnet/NetworkInterface_winXP.c line 256:

> 254: 
> 255: ret = enumInterfaces(env, netifPP);
> 256: if (ret == -1) {

this change is questionable: enumInterfaces returns -2  to allows processing to 
continue with IPv6 data ... this change will prohibit that policy

-

PR: https://git.openjdk.java.net/jdk/pull/6090

Re: RFR: 8278961: Enable debug logging in java/net/DatagramSocket/SendDatagramToBadAddress.java

[Mark Sheppard]

On Mon, 20 Dec 2021 08:51:17 GMT, Daniel Jeliński  wrote:

>> Here too I didn't want to change the current behaviour/code of the test. 
>> It's not just this catch block but even the one a few lines above which 
>> catches `InterruptedIOException`. Neither the `send()` nor the `receive()` 
>> APIs of `DatagramSocket` specify that they throw this specific exception, 
>> but the test seems to catch it and consider it a socket timeout. So I'm 
>> guessing this test and the catch block was written with some specific 
>> context in mind and I didn't want to change that part as part of this PR 
>> which only aims to enable logging by default. Plus like Mark notes, the spec 
>> says `PortUnreachableException` "may" be thrown and isn't guaranteed.
>
> Right. Looks like `OSsupportsFeature` was used to filter out systems that 
> don't send "port unreachable"; we could probably use it here if the failure 
> turns out to be OS-specific.
> 
> Additional logs look fine.

as I indicated below, we have tracked and investigated this issue and it is not 
purely a macosx-aarch64 issue. It may also be a  test env issue. As such using 
the OS filtering in the test, which is designed to primarily exclued AIX, 
wouldn't be the appropraite approach. If we wish to exclude the test on 
macosx-aarch64, then use of Problemlist is more appropriate.

-

PR: https://git.openjdk.java.net/jdk/pull/6883

Re: RFR: 8278961: Enable debug logging in java/net/DatagramSocket/SendDatagramToBadAddress.java

[Mark Sheppard]

On Fri, 17 Dec 2021 14:52:53 GMT, Jaikiran Pai  wrote:

> Can I please get a review for this test only change which proposes to enable 
> debug logs from the test that failed intermittently? This change addresses 
> https://bugs.openjdk.java.net/browse/JDK-8278961.
> 
> The change passes the (test specific) `-d` option to enable logs from that 
> test by default. While I was at it, I even added a few more debug logs hoping 
> it might provide some hints if/when it fails next.
> 
> For reference, a (successful) run of this test will now print something like:
> 
> 
> --System.out:(18/930)--
> running on OS that supports ICMP port unreachable
> Testing with class java.net.DatagramSocket
> tests will be run against destination address localhost/127.0.0.1 port 52682
> Checking send to connected address ...
> socket is locally bound to address /127.0.0.1 port 52681
> received data from address /127.0.0.1 port 52681
> received data from address /127.0.0.1 port 52681
> received data from address /127.0.0.1 port 52681
> received data from address /127.0.0.1 port 52681
> received data from address /127.0.0.1 port 52681
> Checking send to non-connected address ...
> received data from address /127.0.0.1 port 52681
> Checking send to invalid address ...
> Got expected exception: java.net.PortUnreachableException
> Got expected exception: java.net.PortUnreachableException
> Got expected exception: java.net.PortUnreachableException
> Got expected exception: java.net.PortUnreachableException
> Got expected exception: java.net.PortUnreachableException

it is a good idea to turn on the diagnostics -- we have used it in private test 
runs.

the current crop of failures are on macos-aarch64, it appears to be an OS 
issue, in that an ICMP destination unreachabale is not being generated for the 
UDP sends to an non existent socket end point. BUT, there is no obligation to 
do so -- it is not mandaotory in the spec, something along the lines:

"If, in the destination host, the IP module cannot deliver the datagram because 
the indicated protocol module or process port is not active, the destination 
host **may** send a destination unreachable message to the source host." 

The question is: "is it a bug or feature in the OS?" Or it could be a host 
confoguration issue. There may be some configuration that has disabled ICMP, 
maybe a firewall policy or security setting in the kerenal, not to generate 
ICMP DST Unreachable responses?
We haven't found one yet.

-

PR: https://git.openjdk.java.net/jdk/pull/6883

Re: RFR: 8277957: Add test group for IPv6 exclusive testing [v2]

[Mark Sheppard]

On Fri, 3 Dec 2021 20:26:46 GMT, Ivan Šipka  wrote:

>> Adding test group for IPv6 exclusive testing.
>
> Ivan Šipka has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   added comment for adding the ipv6_only  testgroup

Marked as reviewed by msheppar (Reviewer).

test/jdk/TEST.groups line 521:

> 519:   
> jdk/jfr/event/gc/objectcount/TestObjectCountAfterGCEventWithG1ConcurrentMark.java
>  \
> 520:   jdk/jfr/event/gc/heapsummary/TestHeapSummaryEventG1.java
> 521: 

a comment to indicate the purpose of this grouping i.e. as discussed earlier

This set of tests will be executed in an ipv6 only environment

-

PR: https://git.openjdk.java.net/jdk/pull/6600

Re: RFR: 8276401: Use blessed modifier order in java.net.http

[Mark Sheppard]

On Wed, 3 Nov 2021 10:11:31 GMT, Daniel Fuchs  wrote:

> Hi, 
> 
> Please find here a trivial cleanup change that updates classes in the 
> `java.net.http` module to use the "blessed modifier order".
> 
> The changeset was obtained by running `sh ./bin/blessed-modifier-order.sh 
> src/java.net.http`.
> 
> best regards,
> 
> -- daniel

> _Mailing list message from [Daniel Fuchs](mailto:daniel.fu...@oracle.com) on 
> [net-dev](mailto:net-...@mail.openjdk.java.net):_
> 
> Hi Mark,
> 
> On 03/11/2021 14:30, Mark Sheppard wrote:
> 
> > a general comment on the static abstract class  changes to abstract static 
> > class. For me the former and current declarations seems more appropriate, 
> > that is, static abstract class,  as the static modifier immediately conveys 
> > a significant and strong structural relationship with outer or containing 
> > class. While abstract has a qualification on class i.e. the type of class 
> > and appearing directly before class is more natural (to me !!). As such, 
> > abstract qualifies the static relationship.
> > The placement and ordering of the modifier should be to assist in covering 
> > semantics when scanning code, and conveys a certain level of "importance" 
> > of the qualifier's semantics
> 
> WRT `static abstract` vs `abstract static` I had exactly the same feeling - 
> but since there is a blessed ordering and a script to fix classes to conform 
> to the blessed ordering I'm not going to fight it.
> 
> best regards,
> 
> -- daniel

:-)  cula bula ... that's fair enough, no argument here, just thought it worth 
a comment.
as with a lot of "style guides", the blessed order can be a little idiosyncratic
a bit like the case of would you wear a stripped tie with a check shirt, or is 
a plain tie the only suitable option :-)

-

PR: https://git.openjdk.java.net/jdk/pull/6228

Re: RFR: 8276401: Use blessed modifier order in java.net.http

[Mark Sheppard]

On Wed, 3 Nov 2021 10:11:31 GMT, Daniel Fuchs  wrote:

> Hi, 
> 
> Please find here a trivial cleanup change that updates classes in the 
> `java.net.http` module to use the "blessed modifier order".
> 
> The changeset was obtained by running `sh ./bin/blessed-modifier-order.sh 
> src/java.net.http`.
> 
> best regards,
> 
> -- daniel

a general comment on the static abstract class  changes to abstract static 
class. For me the former and current declarations seems more appropriate, that 
is, static abstract class,  as the static modifier immediately conveys a 
significant and strong structural relationship with outer or containing class. 
While abstract has a qualification on class i.e. the type of class and 
appearing directly before class is more natural (to me !!). As such, abstract 
qualifies the static relationship.
The placement and ordering of the modifier should be to assist in covering 
semantics when scanning code, and conveys a certain level of "importance" of 
the qualifier's semantics

-

PR: https://git.openjdk.java.net/jdk/pull/6228

Re: RFR: 8276348: Use blessed modifier order in java.base

[Mark Sheppard]

On Tue, 2 Nov 2021 18:17:36 GMT, Pavel Rappo  wrote:

>> It's tough when a natural language clashes with a programming language. I 
>> appreciate that this particular clash might cause discomfort to native 
>> English speakers. (This reminds me of that _DOSASCOMP_ mnemonic for 
>> adjective order.) That said, consider the following pragmatic aspect. Unless 
>> we change the script not to process prose in comments (btw, how would we do 
>> that?), every single time we run that script, that particular line in 
>> Object.java will need to be tracked and excluded.
>
> Here's a bit of archaeology. I found the original JDK-8136583 patch, which 
> has moved from where it was in the RFR to 
> https://cr.openjdk.java.net/~martin/webrevs/jdk9/blessed-modifier-order/. 
> That patch doesn't change Object.java. The RFR thread mentions neither that 
> javadoc change nor any javadoc change for that matter. So either the script 
> was different, or Martin filtered that line (from Object.java) out before 
> creating the webrev.  
> 
> Now, in his followup thread on core-libs-dev, 
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-September/035273.html,
>  Martin specifically pointed out javadoc changes and said that they seem fine 
> to him.

"to each his own". I think static synchronized reads best and more natural  
than synchronzied static. Also from a semantic point of view as it conveys 
class level characteristic thus lends static to having a prominence in 
specified order.

-

PR: https://git.openjdk.java.net/jdk/pull/6213

Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v3]

[Mark Sheppard]

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]

[Mark Sheppard]

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

[jdk17] Integrated: JDK-8265369 [macos-aarch64] java/net/MulticastSocket/Promiscuous.java failed with "SocketException: Cannot allocate memory"

[Mark Sheppard]

On Mon, 14 Jun 2021 15:28:01 GMT, Mark Sheppard  wrote:

> JDK-8265369 [macos-aarch64] java/net/MulticastSocket/Promiscuous.java failed 
> with "SocketException: Cannot allocate memory"
> 
> The test java/net/MulticastSocket/Promiscuous.java has been observed to fail 
> on a regular basis on macosx-aarch.
> This is typically under heavy test load on a test machine. Analysis of the 
> problem have
> shown that the setsockopt for joining a multicast group will intermittently 
> fail with ENOMEM.
> 
> While analysis of test environment shows significant memory usage and some 
> memory pressure, it is
> not excessive and as such it is deemed transition or temporary condition, 
> such that a retry of the
> setsockopt system call, has been seen to mitigate the issue. This adds to the 
> stability of the
> Promiscuous.java test and reduces test failure noise.
> 
> The proposed fix is in 
> open/src/java.base/unix/native/libnet/PlainDatagramSocketImpl.c
> in the mcast_join_leave function.  That is, if setsockopt to join an mcast 
> group fails, and the errno == ENOMEM, 
> then re-invoke the setsockopt system call for joining a mcast group.
> The change has been applied as a conditional compilation.
> Additionally this change result in the Promiscuous.java test being removed 
> from the
> ProblemList.txt.
> 
> Please oblige and review the changes for a fix of the issue JDK-8265369

This pull request has now been integrated.

Changeset: d8a0582a
Author:Mark Sheppard 
URL:   
https://git.openjdk.java.net/jdk17/commit/d8a0582a36340bcc65910f3a34132ec6e04e5d01
Stats: 22 lines in 2 files changed: 16 ins; 1 del; 5 mod

8265369: [macos-aarch64] java/net/MulticastSocket/Promiscuous.java failed with 
"SocketException: Cannot allocate memory"

Reviewed-by: dfuchs, michaelm, chegar

-

PR: https://git.openjdk.java.net/jdk17/pull/44

Re: [jdk17] RFR: JDK-8265369 [macos-aarch64] java/net/MulticastSocket/Promiscuous.java failed with "SocketException: Cannot allocate memory" [v3]

[Mark Sheppard]

> JDK-8265369 [macos-aarch64] java/net/MulticastSocket/Promiscuous.java failed 
> with "SocketException: Cannot allocate memory"
> 
> The test java/net/MulticastSocket/Promiscuous.java has been observed to fail 
> on a regular basis on macosx-aarch.
> This is typically under heavy test load on a test machine. Analysis of the 
> problem have
> shown that the setsockopt for joining a multicast group will intermittently 
> fail with ENOMEM.
> 
> While analysis of test environment shows significant memory usage and some 
> memory pressure, it is
> not excessive and as such it is deemed transition or temporary condition, 
> such that a retry of the
> setsockopt system call, has been seen to mitigate the issue. This adds to the 
> stability of the
> Promiscuous.java test and reduces test failure noise.
> 
> The proposed fix is in 
> open/src/java.base/unix/native/libnet/PlainDatagramSocketImpl.c
> in the mcast_join_leave function.  That is, if setsockopt to join an mcast 
> group fails, and the errno == ENOMEM, 
> then re-invoke the setsockopt system call for joining a mcast group.
> The change has been applied as a conditional compilation.
> Additionally this change result in the Promiscuous.java test being removed 
> from the
> ProblemList.txt.
> 
> Please oblige and review the changes for a fix of the issue JDK-8265369

Mark Sheppard has updated the pull request incrementally with one additional 
commit since the last revision:

  JDK-8265369 [macos-aarch64] java/net/MulticastSocket/Promiscuous.java failed 
with "SocketException: Cannot allocate memory"
  remove #ifdef __APPLE__ from int res; declaration, remove space and fix 
indentation (as per comments from CH and M3)

-

Changes:
  - all: https://git.openjdk.java.net/jdk17/pull/44/files
  - new: https://git.openjdk.java.net/jdk17/pull/44/files/ac82b8df..3365a31c

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk17=44=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk17=44=01-02

  Stats: 4 lines in 1 file changed: 0 ins; 2 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk17/pull/44.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/44/head:pull/44

PR: https://git.openjdk.java.net/jdk17/pull/44

Re: [jdk17] RFR: JDK-8265369 [macos-aarch64] java/net/MulticastSocket/Promiscuous.java failed with "SocketException: Cannot allocate memory" [v2]

[Mark Sheppard]

On Thu, 17 Jun 2021 12:39:17 GMT, Michael McMahon  wrote:

>> src/java.base/unix/native/libnet/PlainDatagramSocketImpl.c line 2112:
>> 
>>> 2110: res = setsockopt(fd, IPPROTO_IPV6, (join ? ADD_MEMBERSHIP : 
>>> DRP_MEMBERSHIP),
>>> 2111:(char *) , sizeof (mname6));
>>> 2112: 
>> 
>> Seems to be an extraneous space after sizeof here.
>
> A general question. Is "__APPLE__" the preferred macro name or MACOSX? Not a 
> big deal but MACOSX looks slightly more common. [Seems github has removed the 
> underscores from APPLE]

thanks ... I'll attend to the spacing 
I used __APPLE__ as it was already in place for a change to set socket buffer 
size

-

PR: https://git.openjdk.java.net/jdk17/pull/44

Re: [jdk17] RFR: JDK-8265369 [macos-aarch64] java/net/MulticastSocket/Promiscuous.java failed with "SocketException: Cannot allocate memory" [v2]

[Mark Sheppard]

On Thu, 17 Jun 2021 11:07:23 GMT, Chris Hegarty  wrote:

>> Mark Sheppard has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   JDK-8265369 [macos-aarch64] java/net/MulticastSocket/Promiscuous.java 
>> failed with "SocketException: Cannot allocate memory"
>>   amendments as per suggestion from Chris Hegarty
>
> src/java.base/unix/native/libnet/PlainDatagramSocketImpl.c line 1843:
> 
>> 1841: int res;
>> 1842: #endif
>> 1843: 
> 
> res will need to be declared unconditionally, no? ( since it is used on all 
> platforms )

yes ... good spot   . as testing has been focused on macos, this error 
didn't show

-

PR: https://git.openjdk.java.net/jdk17/pull/44

Re: [jdk17] RFR: JDK-8265369 [macos-aarch64] java/net/MulticastSocket/Promiscuous.java failed with "SocketException: Cannot allocate memory" [v2]

[Mark Sheppard]

> JDK-8265369 [macos-aarch64] java/net/MulticastSocket/Promiscuous.java failed 
> with "SocketException: Cannot allocate memory"
> 
> The test java/net/MulticastSocket/Promiscuous.java has been observed to fail 
> on a regular basis on macosx-aarch.
> This is typically under heavy test load on a test machine. Analysis of the 
> problem have
> shown that the setsockopt for joining a multicast group will intermittently 
> fail with ENOMEM.
> 
> While analysis of test environment shows significant memory usage and some 
> memory pressure, it is
> not excessive and as such it is deemed transition or temporary condition, 
> such that a retry of the
> setsockopt system call, has been seen to mitigate the issue. This adds to the 
> stability of the
> Promiscuous.java test and reduces test failure noise.
> 
> The proposed fix is in 
> open/src/java.base/unix/native/libnet/PlainDatagramSocketImpl.c
> in the mcast_join_leave function.  That is, if setsockopt to join an mcast 
> group fails, and the errno == ENOMEM, 
> then re-invoke the setsockopt system call for joining a mcast group.
> The change has been applied as a conditional compilation.
> Additionally this change result in the Promiscuous.java test being removed 
> from the
> ProblemList.txt.
> 
> Please oblige and review the changes for a fix of the issue JDK-8265369

Mark Sheppard has updated the pull request incrementally with one additional 
commit since the last revision:

  JDK-8265369 [macos-aarch64] java/net/MulticastSocket/Promiscuous.java failed 
with "SocketException: Cannot allocate memory"
  amendments as per suggestion from Chris Hegarty

-

Changes:
  - all: https://git.openjdk.java.net/jdk17/pull/44/files
  - new: https://git.openjdk.java.net/jdk17/pull/44/files/2e940bc0..ac82b8df

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk17=44=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk17=44=00-01

  Stats: 58 lines in 1 file changed: 9 ins; 34 del; 15 mod
  Patch: https://git.openjdk.java.net/jdk17/pull/44.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/44/head:pull/44

PR: https://git.openjdk.java.net/jdk17/pull/44

Re: [jdk17] RFR: JDK-8265369 [macos-aarch64] java/net/MulticastSocket/Promiscuous.java failed with "SocketException: Cannot allocate memory"

[Mark Sheppard]

On Tue, 15 Jun 2021 08:58:45 GMT, Chris Hegarty  wrote:

>> JDK-8265369 [macos-aarch64] java/net/MulticastSocket/Promiscuous.java failed 
>> with "SocketException: Cannot allocate memory"
>> 
>> The test java/net/MulticastSocket/Promiscuous.java has been observed to fail 
>> on a regular basis on macosx-aarch.
>> This is typically under heavy test load on a test machine. Analysis of the 
>> problem have
>> shown that the setsockopt for joining a multicast group will intermittently 
>> fail with ENOMEM.
>> 
>> While analysis of test environment shows significant memory usage and some 
>> memory pressure, it is
>> not excessive and as such it is deemed transition or temporary condition, 
>> such that a retry of the
>> setsockopt system call, has been seen to mitigate the issue. This adds to 
>> the stability of the
>> Promiscuous.java test and reduces test failure noise.
>> 
>> The proposed fix is in 
>> open/src/java.base/unix/native/libnet/PlainDatagramSocketImpl.c
>> in the mcast_join_leave function.  That is, if setsockopt to join an mcast 
>> group fails, and the errno == ENOMEM, 
>> then re-invoke the setsockopt system call for joining a mcast group.
>> The change has been applied as a conditional compilation.
>> Additionally this change result in the Promiscuous.java test being removed 
>> from the
>> ProblemList.txt.
>> 
>> Please oblige and review the changes for a fix of the issue JDK-8265369
>
> src/java.base/unix/native/libnet/PlainDatagramSocketImpl.c line 2017:
> 
>> 2015: }
>> 2016: } else {
>> 2017: #endif
> 
> The handling of ENOMEM here is consistent with how it is handled in the NIO 
> area - good.
> 
> I wonder if a little restructuring may simplify the call flow? For example, 
> something similar to:
> 
>   int n;
>   ...
>   /*
>* Join the multicast group.
>*/
>n = setsockopt(fd, IPPROTO_IP, (join ? 
> IP_ADD_MEMBERSHIP:IP_DROP_MEMBERSHIP),
>   (char *) , mname_len);
> #ifdef __APPLE__
>// workaround macOS bug where IP_ADD/DROP_MEMBERSHIP fails intermittently
>if (n < 0 && errno == ENOMEM) {
>   n = setsockopt(fd, IPPROTO_IP, (join ? 
> IP_ADD_MEMBERSHIP:IP_DROP_MEMBERSHIP),
>   (char *) , mname_len);
>}
> #endif
> 
>   if (n < 0) {  ...

yes, I'll do that ... originally I did this but reverted to current change to 
retain the current structure of the file
thanks for the suggestion, it is much neater

-

PR: https://git.openjdk.java.net/jdk17/pull/44

[jdk17] RFR: JDK-8265369 [macos-aarch64] java/net/MulticastSocket/Promiscuous.java failed with "SocketException: Cannot allocate memory"

[Mark Sheppard]

… failed with "SocketException: Cannot allocate memory

The test java/net/MulticastSocket/Promiscuous.java has been observed to fail on 
a regular basis on macosx-aarch.
This is typically under heavy test load on a test machine. Analysis of the 
problem have
shown that the setsockopt for joining a multicast group will intermittently 
fail with ENOMEM.

While analysis of test environment shows significant memory usage and some 
memory pressure, it is
not excessive and as such it is deemed transition or temporary condition, such 
that a retry of the
setsockopt system call, has been seen to mitigate the issue. This adds to the 
stability of the
Promiscuous.java test and reduces test failure noise.

The proposed fix is in 
open/src/java.base/unix/native/libnet/PlainDatagramSocketImpl.c
in the mcast_join_leave function.  That is, if setsockopt to join an mcast 
group fails, and the errno == ENOMEM, 
then re-invoke the setsockopt system call for joining a mcast group.
The change has been applied as a conditional compilation.
Additionally this change result in the Promiscuous.java test being removed from 
the
ProblemList.txt.

Please oblige and review the changes for a fix of the issue JDK-8265369

-

Commit messages:
 - JDK-8265369 [macos-aarch64] java/net/MulticastSocket/Promiscuous.java failed 
with "SocketException: Cannot allocate memory"
 - JDK-8265369 [macos-aarch64] java/net/MulticastSocket/Promiscuous.java failed 
with "SocketException: Cannot allocate memory"
 - JDK-8265369 [macos-aarch64] java/net/MulticastSocket/Promiscuous.java failed 
with "SocketException: Cannot allocate memory

Changes: https://git.openjdk.java.net/jdk17/pull/44/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk17=44=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8265369
  Stats: 51 lines in 2 files changed: 43 ins; 1 del; 7 mod
  Patch: https://git.openjdk.java.net/jdk17/pull/44.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/44/head:pull/44

PR: https://git.openjdk.java.net/jdk17/pull/44

Re: RFR: 8267353: java/net/SctpSanity.java fails due to Protocol not supported

[Mark Sheppard]

On Wed, 26 May 2021 03:54:14 GMT, Jie Fu  wrote:

> Hi all,
> 
> java/net/SctpSanity.java fails on some of our test machines due to Protocol 
> not supported.
> The reason is that the test fails to detect all the cases when a machine 
> doesn't support SCTP.
> 
> The fix just follows what are done in [1][2][3] to detect unsupported 
> paltforms at the beginning.
> 
> Thanks.
> Best regards,
> Jie
> 
> [1] 
> https://github.com/openjdk/jdk/blob/master/test/jdk/com/sun/nio/sctp/SctpMultiChannel/Util.java#L59
> [2] 
> https://github.com/openjdk/jdk/blob/master/test/jdk/com/sun/nio/sctp/SctpServerChannel/Util.java#L59
> [3] 
> https://github.com/openjdk/jdk/blob/master/test/jdk/com/sun/nio/sctp/SctpChannel/Util.java#L59

if sctp is configured on a linux system would there exist a  /proc/net/sctp  
and have an entry in /proc/net/protocols ?

-

PR: https://git.openjdk.java.net/jdk/pull/4199

Integrated: JDK-8265362 java/net/Socket/UdpSocket.java fails with "java.net.BindException: Address already in use" (macos-aarch64)

[Mark Sheppard]

On Tue, 18 May 2021 22:43:14 GMT, Mark Sheppard  wrote:

> The test java/net/Socket/UdpSocket.java has been seen to fail with a 
> BindException, in the testMaxSockets test, on a regular basis on 
> macOS-aarch64 platform. testMaxSockets tests the maximum number of UDP 
> Sockets that may be created as defined by a system property 
> sun.net.maxDatagramSockets. It invokes the Socket constructor 
> Socket(InetAddress host, int port, boolean stream) with stream set to false 
> to create a UDP Socket. This instantiation is a compound operation, 
> consisting of the creation of a socket, the explicit binding of wildcard 
> address and ephemeral port, and a connect to the socket end point specified 
> in the constructor parameters.  Analysis has shown that during the test that 
> the OS intermittently allocates the same ephemeral port multiple times during 
> the bind system call, such that two separate sockets end up bound to the same 
> port. Then on the connect invocation a BindException is thrown for the second 
> socket. This has been determined to be a transient condition duri
 ng heavy loads and where a significant number of ephemeral ports are being 
allocated.
> 
> As this is deemed an OS issues, and in order to increase test stability, it 
> has been found that for the BindException condition a retry of the Socket 
> creation mitigates the issues and tests the max creation property.

This pull request has now been integrated.

Changeset: bb085f68
Author:Mark Sheppard 
URL:   
https://git.openjdk.java.net/jdk/commit/bb085f684d1154ffd6b2169259c67cfb19958380
Stats: 14 lines in 2 files changed: 10 ins; 3 del; 1 mod

8265362: java/net/Socket/UdpSocket.java fails with "java.net.BindException: 
Address already in use" (macos-aarch64)

Reviewed-by: dfuchs, alanb

-

PR: https://git.openjdk.java.net/jdk/pull/4103

Re: RFR: JDK-8265362 java/net/Socket/UdpSocket.java fails with "java.net.BindException: Address already in use" (macos-aarch64) [v2]

[Mark Sheppard]

On Mon, 24 May 2021 14:34:54 GMT, Mark Sheppard  wrote:

>> Mark Sheppard has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   JDK-8265362 java/net/Socket/UdpSocket.java fails with 
>> "java.net.BindException: Address already in use" (macos-aarch64)
>>   update as per request by AB
>
> I used stdout for convenience as it aligns with the  two other output 
> statements, so that I see immediately the retry output as follows:
> 
> --System.out:(12/349)--
> [TestNG] Running:
>   java/net/Socket/UdpSocket.java
> 
> BindException caught retry Socket creation
> test UdpSocket.testMaxSockets(): success
> test UdpSocket.testSendReceive(): success
> 
> ===

I could have used return directly in multiple places ... but my style 
preference is a single exit point from a method

-

PR: https://git.openjdk.java.net/jdk/pull/4103

Re: RFR: JDK-8265362 java/net/Socket/UdpSocket.java fails with "java.net.BindException: Address already in use" (macos-aarch64) [v2]

[Mark Sheppard]

On Mon, 24 May 2021 12:30:38 GMT, Mark Sheppard  wrote:

>> The test java/net/Socket/UdpSocket.java has been seen to fail with a 
>> BindException, in the testMaxSockets test, on a regular basis on 
>> macOS-aarch64 platform. testMaxSockets tests the maximum number of UDP 
>> Sockets that may be created as defined by a system property 
>> sun.net.maxDatagramSockets. It invokes the Socket constructor 
>> Socket(InetAddress host, int port, boolean stream) with stream set to false 
>> to create a UDP Socket. This instantiation is a compound operation, 
>> consisting of the creation of a socket, the explicit binding of wildcard 
>> address and ephemeral port, and a connect to the socket end point specified 
>> in the constructor parameters.  Analysis has shown that during the test that 
>> the OS intermittently allocates the same ephemeral port multiple times 
>> during the bind system call, such that two separate sockets end up bound to 
>> the same port. Then on the connect invocation a BindException is thrown for 
>> the second socket. This has been determined to be a transient condition dur
 ing heavy loads and where a significant number of ephemeral ports are being 
allocated.
>> 
>> As this is deemed an OS issues, and in order to increase test stability, it 
>> has been found that for the BindException condition a retry of the Socket 
>> creation mitigates the issues and tests the max creation property.
>
> Mark Sheppard has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   JDK-8265362 java/net/Socket/UdpSocket.java fails with 
> "java.net.BindException: Address already in use" (macos-aarch64)
>   update as per request by AB

I used stdout for convenience as it aligns with the  two other output 
statements, so that I see immediately the retry output as follows:

--System.out:(12/349)--
[TestNG] Running:
  java/net/Socket/UdpSocket.java

BindException caught retry Socket creation
test UdpSocket.testMaxSockets(): success
test UdpSocket.testSendReceive(): success

===

-

PR: https://git.openjdk.java.net/jdk/pull/4103

Re: RFR: JDK-8265362 java/net/Socket/UdpSocket.java fails with "java.net.BindException: Address already in use" (macos-aarch64) [v2]

[Mark Sheppard]

On Sat, 22 May 2021 10:54:06 GMT, Mark Sheppard  wrote:

>> BTW: Is one retry enough? There is at least one other replace where we've 
>> had to retry to workaround a macOS bug and one retry was enough there too.
>
> I have submitted a significant number of MACH5 job runs with repeat mode over 
> the past week - approx  50 x 50,  and observed the retry to have been 
> triggered about 5 times, this gives some level of assurance that test 
> stability will exist. But CI builds will exercise this a lot more.

updated as requested newUdpSocket --> s, and biEx --> unexpected

-

PR: https://git.openjdk.java.net/jdk/pull/4103

Re: RFR: JDK-8265362 java/net/Socket/UdpSocket.java fails with "java.net.BindException: Address already in use" (macos-aarch64) [v2]

[Mark Sheppard]

> The test java/net/Socket/UdpSocket.java has been seen to fail with a 
> BindException, in the testMaxSockets test, on a regular basis on 
> macOS-aarch64 platform. testMaxSockets tests the maximum number of UDP 
> Sockets that may be created as defined by a system property 
> sun.net.maxDatagramSockets. It invokes the Socket constructor 
> Socket(InetAddress host, int port, boolean stream) with stream set to false 
> to create a UDP Socket. This instantiation is a compound operation, 
> consisting of the creation of a socket, the explicit binding of wildcard 
> address and ephemeral port, and a connect to the socket end point specified 
> in the constructor parameters.  Analysis has shown that during the test that 
> the OS intermittently allocates the same ephemeral port multiple times during 
> the bind system call, such that two separate sockets end up bound to the same 
> port. Then on the connect invocation a BindException is thrown for the second 
> socket. This has been determined to be a transient condition duri
 ng heavy loads and where a significant number of ephemeral ports are being 
allocated.
> 
> As this is deemed an OS issues, and in order to increase test stability, it 
> has been found that for the BindException condition a retry of the Socket 
> creation mitigates the issues and tests the max creation property.

Mark Sheppard has updated the pull request incrementally with one additional 
commit since the last revision:

  JDK-8265362 java/net/Socket/UdpSocket.java fails with 
"java.net.BindException: Address already in use" (macos-aarch64)
  update as per request by AB

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4103/files
  - new: https://git.openjdk.java.net/jdk/pull/4103/files/1f05203e..87cd9c5e

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4103=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4103=00-01

  Stats: 5 lines in 1 file changed: 0 ins; 0 del; 5 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4103.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4103/head:pull/4103

PR: https://git.openjdk.java.net/jdk/pull/4103

Re: RFR: JDK-8265362 java/net/Socket/UdpSocket.java fails with "java.net.BindException: Address already in use" (macos-aarch64)

[Mark Sheppard]

On Sat, 22 May 2021 06:46:18 GMT, Alan Bateman  wrote:

>> Thanks, and if you want to keep it consistent with the existing code then 
>> you could rename "Socket newUdpSocket" and "biEx", or just change it to 
>> "return new Socket(...)".
>
> BTW: Is one retry enough? There is at least one other replace where we've had 
> to retry to workaround a macOS bug and one retry was enough there too.

I have submitted a significant number of MACH5 job runs with repeat mode over 
the past week - approx  50 x 50,  and observed the retry to have been triggered 
about 5 times, this gives some level of assurance that test stability will 
exist. But CI builds will exercise this a lot more.

-

PR: https://git.openjdk.java.net/jdk/pull/4103

RFR: JDK-8265362 java/net/Socket/UdpSocket.java fails with "java.net.BindException: Address already in use" (macos-aarch64)

[Mark Sheppard]

The test java/net/Socket/UdpSocket.java has been seen to fail with a 
BindException, in the testMaxSockets test, on a regular basis on macOS-aarch64 
platform. testMaxSockets tests the maximum number of UDP Sockets that may be 
created as defined by a system property sun.net.maxDatagramSockets. It invokes 
the Socket constructor Socket(InetAddress host, int port, boolean stream) with 
stream set to false to create a UDP Socket. This instantiation is a compound 
operation, consisting of the creation of a socket, the explicit binding of 
wildcard address and ephemeral port, and a connect to the socket end point 
specified in the constructor parameters.  Analysis has shown that during the 
test that the OS intermittently allocates the same ephemeral port multiple 
times during the bind system call, such that two separate sockets end up bound 
to the same port. Then on the connect invocation a BindException is thrown for 
the second socket. This has been determined to be a transient condition during
  heavy loads and where a significant number of ephemeral ports are being 
allocated.

As this is deemed an OS issues, and in order to increase test stability, it has 
been found that for the BindException condition a retry of the Socket creation 
mitigates the issues and tests the max creation property.

-

Commit messages:
 - JDK-8265362 java/net/Socket/UdpSocket.java fails with 
"java.net.BindException: Address already in use" (macos-aarch64)
 - JDK-8265362: additions to execute a retry of UDP Socket construction if a 
BindException thrown during the testMaxSockets test

Changes: https://git.openjdk.java.net/jdk/pull/4103/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4103=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8265362
  Stats: 14 lines in 2 files changed: 10 ins; 3 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4103.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4103/head:pull/4103

PR: https://git.openjdk.java.net/jdk/pull/4103

Re: RFR: JDK-8265362 java/net/Socket/UdpSocket.java fails with "java.net.BindException: Address already in use" (macos-aarch64)

[Mark Sheppard]

On Wed, 19 May 2021 05:56:07 GMT, Alan Bateman  wrote:

>> The test java/net/Socket/UdpSocket.java has been seen to fail with a 
>> BindException, in the testMaxSockets test, on a regular basis on 
>> macOS-aarch64 platform. testMaxSockets tests the maximum number of UDP 
>> Sockets that may be created as defined by a system property 
>> sun.net.maxDatagramSockets. It invokes the Socket constructor 
>> Socket(InetAddress host, int port, boolean stream) with stream set to false 
>> to create a UDP Socket. This instantiation is a compound operation, 
>> consisting of the creation of a socket, the explicit binding of wildcard 
>> address and ephemeral port, and a connect to the socket end point specified 
>> in the constructor parameters.  Analysis has shown that during the test that 
>> the OS intermittently allocates the same ephemeral port multiple times 
>> during the bind system call, such that two separate sockets end up bound to 
>> the same port. Then on the connect invocation a BindException is thrown for 
>> the second socket. This has been determined to be a transient condition dur
 ing heavy loads and where a significant number of ephemeral ports are being 
allocated.
>> 
>> As this is deemed an OS issues, and in order to increase test stability, it 
>> has been found that for the BindException condition a retry of the Socket 
>> creation mitigates the issues and tests the max creation property.
>
> test/jdk/java/net/Socket/UdpSocket.java line 151:
> 
>> 149: }
>> 150: }
>> 151: return newUdpSocket;
> 
> I added this test in advance of JEP 353 as we didn't have much coverage for 
> this deprecated constructor. No objection to the retry if it helps.  Is the 
> catching of SocketException a left over from testing? I assume the patch can 
> be simplified down to:
> 
> 
> try {
>return new Socket(InetAddress.getLoopbackAddress(), 8000, false);
> } catch (BindException e) {
> System.out.println(...);
>return new Socket(InetAddress.getLoopbackAddress(), 8000, false);
> }

yes, thanks for that  ... updated as requested

-

PR: https://git.openjdk.java.net/jdk/pull/4103

Re: RFR: 8263233: Update java.net and java.nio to use instanceof pattern variable [v3]

[Mark Sheppard]

On Tue, 9 Mar 2021 20:26:19 GMT, Daniel Fuchs  wrote:

>> src/java.base/share/classes/java/net/NetMulticastSocket.java line 219:
>> 
>>> 217: if (addr == null)
>>> 218: addr = new InetSocketAddress(0);
>>> 219: if (!(addr instanceof InetSocketAddress epoint))
>> 
>> in the context of this type of change the negative logic is a little obtuse 
>> or disjoint in the new style variable declaration, and its subsequent use. 
>> I'd find it a more natural flow and easier to read with the instanceof 
>> pattern prefacing a block in which the variable is used
>> if (addr instanceof InetSocketAddress epoint) {
>>   // a block which is using the epoint variable
>>if (epoint.isUnresolved)
>>   throw new SocketException("Unresolved address");
>>InetAddress iaddr = epoint.getAddress;
>>  etc, etc ...
>> } else {
>>throw new IllegalArgumentException("Unsupported address type!");
>> }
>
> The disadvantage is that it would add another level of braces.

maybe ... I find it would provide an easier flow to code  ... an "easy on the 
eye" call flow linkage - the variable is declared and then used   instead 
of the disconnect of throwing an exception or non immediate use of the 
variable. 

but sure,  isn't code somewhat idiosyncratic!!

-

PR: https://git.openjdk.java.net/jdk/pull/2890

Re: RFR: 8263233: Update java.net and java.nio to use instanceof pattern variable [v3]

[Mark Sheppard]

On Tue, 9 Mar 2021 19:56:25 GMT, Patrick Concannon  
wrote:

>> Hi,
>> 
>> Could someone please review my code for updating the code in the `java.net` 
>> and `java.nio` packages to make use of the `instanceof` pattern variable?
>> 
>> Kind regards,
>> Patrick
>
> Patrick Concannon has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8263233: Refactored equals method further

src/java.base/share/classes/java/net/NetMulticastSocket.java line 219:

> 217: if (addr == null)
> 218: addr = new InetSocketAddress(0);
> 219: if (!(addr instanceof InetSocketAddress epoint))

in the context of this type of change the negative logic is a little obtuse or 
disjoint in the new style variable declaration, and its subsequent use. I'd 
find it a more natural flow and easier to read with the instance pattern 
prefacing a block in which the variable is used
if (addr instanced InetSocketAddress epoint) {
  // a block which is using the epoint variable
   if (epoint.isUnresolved)
  throw new SocketException("Unresolved address");
   InetAddress iaddr = epoint.getAddress;
 etc, etc ...
} else {
   throw new IllegalArgumentException("Unsupported address type!");
}

-

PR: https://git.openjdk.java.net/jdk/pull/2890

Re: RFR: 8259628: jdk/net/ExtendedSocketOption/AsynchronousSocketChannelNAPITest.java fails intermittently [v4]

[Mark Sheppard]

On Fri, 22 Jan 2021 14:41:44 GMT, Daniel Fuchs  wrote:

>> Patrick Concannon has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   8259628: Removed buffer check
>
> You could still do some checking if you wanted.
> If you know that you have written `nsent` bytes, and that you later read 
> `nread` bytes, then you could assert the following:
> 1. nread <= nsent
> 2. writeBuffer.mismatch(readBuffer) == (nread == nsent ? -1 : nread)

Hi Daniel,
   by adding a read/send bytes checks, are we not obscuring the purpose of the 
test and that's the NAPI check ?
Consider that, as the communication channel between the writer and sender is a 
stream, there are no (atomic) message boundaries, and in theory a read could be 
less than an individual write in one iteration,  and then greater than the 
individual write in another iteration. Thus, adding an assert could give rise 
to unnecessary failures - it is unlikely in this test - in fact because of 
colocation you could place a strong bet that it won't happen. 
Nonetheless, the following is possible
iteration N  write n bytes, reads m where  m < n (The underlying TCP 
protocol machine has sent m bytes and not the full n)
iteration N +1  write n bytes, read n + (n-m)  i.e. the current write + the 
residue from the write in iteration N

and then an  assert nread <= nsent will fail  ?

In any case, the main thing is that the test is happy with its NAPI across the 
various reads

best regards
Mark

-

PR: https://git.openjdk.java.net/jdk/pull/2162

Re: RFR: 8259628: jdk/net/ExtendedSocketOption/AsynchronousSocketChannelNAPITest.java fails intermittently

[Mark Sheppard]

On Wed, 20 Jan 2021 12:53:34 GMT, Daniel Fuchs  wrote:

>> Hi,
>> 
>> Could someone please review my fix for JDK-8259628: 
>> '`jdk/net/ExtendedSocketOption/AsynchronousSocketChannelNAPITest.java` fails 
>> intermittently' ?
>> 
>> `AsynchronousSocketChannelNAPITest` is failing intermittently on Linux due 
>> to a race condition caused by not correctly waiting for the result of an 
>> asynchronous operation. This fix rectifies this issue and adds additional 
>> checks to ensure correct result is received.
>> 
>> Kind regards,
>> Patrick
>
> LGTM. Thanks Patrick for taking this on!

This response maybe a bit of overkill or overthinking, but bear with me and 
apologies in advance!!
I think some of the additions have a few (theoretical) consequences - the read 
buffer and write buffer asserts cannot be assumed to be true as this, afaik, is 
not a datagram communication scenario ?? As such read and writes are not atomic.

Additionally they (may) distract from the main semantics of the test, that is, 
the read NAPI remain constant across reads.

The test has been modified to assert some conditions on the read and write 
buffer. These asserts, in theory, can not be assumed to hold true. Consider 
that the write returns a Future and will inform the writer how many bytes have 
been written. So writes are not atomic.
Additionally, as such,  a read may not actually read the number of bytes 
written by the writer, 
due to the underlying semantics of the supporting protocol (TCP/IP). If it was 
UDP as the underlying protocol then the read/write assertions would hold.
Because it is a co-located writer/reader test, and the size of the data 
transfer is small, the likelihood of any variation between the write and read 
is small.
So, do you really need to do the read/write buffer checks. Adding the get() 
method to the Future returned by the read should be sufficient to obviate the 
ReadPendingException

One other minor point: Is the assignment tempID = clientID;  needed for each 
iteration of the loop. The clientID should be constant across all reads.  If 
tempID was renamed originalClientID and assigned in the initialRun block

 if (initialRun) {
assertTrue(clientID >= 0, 
"AsynchronousSocketChannel: Receiver");
initialRun = false;
originalClientID = clientID
} else {
assertEquals(clientID, originalClientID);
}

-

PR: https://git.openjdk.java.net/jdk/pull/2162

Re: RFR: 8252304: Seed an HttpRequest.Builder from an existing HttpRequest [v3]

[Mark Sheppard]

On Mon, 9 Nov 2020 18:38:33 GMT, Mark Sheppard  wrote:

>> I wonder if the spec should be a little more specific than just "seeded" 
>> which I think is fine for the first sentence. But maybe say something like 
>> "fields are copied from the given HttpRequest" in the description.
>
> a couple of points:
> Should the spec state explicitly that an NPE is thrown if the supplied 
> HttpRequest reference is null?
> 
> Alternatively, if the supplied HttpRequest is null then why not return an 
> empty builder?
> 
> I would favour a slightly more verbose statement in the spec, something like:
> 
>/**
>  * Creates a {@code Builder} with its pre request state set from that of 
> an existing {@code HttpRequest}.
>  *
>  *  This method can be used as the basis for building a new request 
> equivalent to a
>  * given request, while allowing further amendment and alteration of 
>  * the pre request state, for example adding additional Http headers, 
> prior to request construction.
>  *
>  * @param request the request
>  * @return a new request builder
>  * @throws IllegalArgumentException if a new builder cannot be seeded from
>  * the given request (for instance, if the request contains 
> illegal
>  * parameters)
>  * @throws NullPointerException is thrown if the supplied HttpRequest is 
> null
>  * @since 16
>  */

Hi Daniel,
   thanks for the clarification   ... I missed that, as I only looked at the 
Builder and HttpRequest descriptions.
So the second question does it seem reasonable to return an empty Builder if 
null is passed as an arg.
However, if an NPE is thrown then it will force the client of the API to be 
more exact in its usage and avoid possible implicit ambuguities with a null 
reference args.

regards
Mark

-

PR: https://git.openjdk.java.net/jdk/pull/1059

Re: RFR: 8252304: Seed an HttpRequest.Builder from an existing HttpRequest [v3]

[Mark Sheppard]

On Mon, 9 Nov 2020 10:28:08 GMT, Michael McMahon  wrote:

>>> /csr
>> 
>> Link to CSR: https://bugs.openjdk.java.net/browse/JDK-8255993
>
> I wonder if the spec should be a little more specific than just "seeded" 
> which I think is fine for the first sentence. But maybe say something like 
> "fields are copied from the given HttpRequest" in the description.

a couple of points:
Should the spec state explicitly that an NPE is thrown if the supplied 
HttpRequest reference is null?

Alternatively, if the supplied HttpRequest is null then why not return an empty 
builder?

I would favour a slightly more verbose statement in the spec, something like:

   /**
 * Creates a {@code Builder} with its pre request state set from that of an 
existing {@code HttpRequest}.
 *
 *  This method can be used as the basis for building a new request 
equivalent to a
 * given request, while allowing further amendment and alteration of 
 * the pre request state, for example adding additional Http headers, prior 
to request construction.
 *
 * @param request the request
 * @return a new request builder
 * @throws IllegalArgumentException if a new builder cannot be seeded from
 * the given request (for instance, if the request contains illegal
 * parameters)
 * @throws NullPointerException is thrown if the supplied HttpRequest is 
null
 * @since 16
 */

-

PR: https://git.openjdk.java.net/jdk/pull/1059

Re: RFR: 8254871: Remove unnecessary string copy in NetworkInterface.c

[Mark Sheppard]

On Tue, 27 Oct 2020 12:12:47 GMT, Michael McMahon  wrote:

>> A small improvement to avoid extra string copy.
>> 
>> [Tests]
>> Jtreg hotspot::hotspot_all_no_apps, jdk::jdk_core and langtools::tier1.
>> No new failure found.
>
> LGTM

In the original the copy would appear to be a way of enforcing that the 
supplied name is not greater than the max IF name size (IFNAMSIZ), 
so in the updated version if limit >= IFNAMSIZ is there any point in performing 
the search? As it is unlikely to match a returned interface name ?

-

PR: https://git.openjdk.java.net/jdk/pull/821

Re: RFC: 8132359: JarURLConnection.getJarFile() resource leak when file is not found

[Mark Sheppard]

Hi Chris, 
sounds good  thanks for the update 


regards 
Mark 



- Original Message - 
From: chris.hega...@oracle.com 
To: akash...@redhat.com, net-dev@openjdk.java.net, mark.shepp...@oracle.com 
Sent: Monday, 15 June, 2020 1:46:06 PM GMT +00:00 GMT Britain, Ireland, 
Portugal 
Subject: Re: RFC: 8132359: JarURLConnection.getJarFile() resource leak when 
file is not found 


Hi Alex, Mark, 


While I think that change for this issue is good, I would like to request that 
we please put it on hold temporarily. 


I will experiment with possible solutions for 8246714 - the URLClassLoader 
issue. It may result in no overlap with this issue, 8132359, but it seems 
prudent to allow that experimentation to happen first. 


Sorry to be asking that this issue wait, I don’t do this lightly or often, but 
it seems like the right thing to do. 


-Chris. 





On 15 Jun 2020, at 13:05, mark sheppard < macanao...@hotmail.com > wrote: 


Hi Alex, 
I think there is some other work planned in this area, 
so it may be best to place this item on hold for a bit. 
There should be an update on this shortly. 


regards 
Mark 





From: Alex Kashchenko < akash...@redhat.com > 
Sent: Monday 15 June 2020 10:35 
To: mark sheppard < macanao...@hotmail.com >; OpenJDK Network Dev list < 
net-dev@openjdk.java.net > 
Cc: Mark Sheppard < mark.shepp...@oracle.com > 
Subject: Re: RFC: 8132359: JarURLConnection.getJarFile() resource leak when 
file is not found 


Hi, 

CSR: https://bugs.openjdk.java.net/browse/JDK-8244650 

On 06/13/2020 10:49 PM, mark sheppard wrote: 
> Hi, 
> For JDK-8132359 it now addresses the issue: 
> 
> Amend JarURLConnection::getJarFile() to return JarFile object reference for 
> nonexistent JAR file entry URL 
> 
> The scenario addressed is that a JarURLConnection is constructed 
> encapsulating a reference to an non existent entry in an extant JarFile, then 
> the getJarFile is reasonably expected to return a reference to the associated 
> JarFile, rather than propagating the IOException thrown by the implicit 
> JarURLConnection::connect invocation in getJarFile. 
> 
> The original test case demonstrates other cross platform issues in this 
> context. But by returning the JarFile reference, ( rather than propagating 
> the exception,) it is then possible to invoke close on the JarFile, and allow 
> the explicit delete of the JarFile. Thus, mitigating a perceived problem on 
> Windows. 
> 
> As such, this is an issue in its own right, and as such it would appear that 
> there is merit in this fix. 

Thanks for your comments! Would it be appropriate to move the CSR to 
"Proposed" [1] at this point? 

> 
> [...] 
> 

[1] https://wiki.openjdk.java.net/display/csr/Main#Main-CSRWorkflows 

-- 
-Alex 

Re: RFC: 8132359: JarURLConnection.getJarFile() resource leak when file is not found

[mark sheppard]

Hi Alex,
   I think there is some other work planned in this area,
so it may be best to place this item on hold for a bit.
There should be an update on this shortly.

regards
Mark


From: Alex Kashchenko 
Sent: Monday 15 June 2020 10:35
To: mark sheppard ; OpenJDK Network Dev list 

Cc: Mark Sheppard 
Subject: Re: RFC: 8132359: JarURLConnection.getJarFile() resource leak when 
file is not found

Hi,

CSR: https://bugs.openjdk.java.net/browse/JDK-8244650

On 06/13/2020 10:49 PM, mark sheppard wrote:
> Hi,
> For JDK-8132359 it now addresses the issue:
>
> Amend JarURLConnection::getJarFile() to return JarFile object reference for 
> nonexistent JAR file entry URL
>
> The scenario addressed is that a JarURLConnection is constructed 
> encapsulating a reference to an non existent entry in an extant JarFile, then 
> the  getJarFile  is reasonably expected to return a reference to the 
> associated JarFile, rather than propagating the IOException thrown by the 
> implicit JarURLConnection::connect invocation  in getJarFile.
>
> The original test case demonstrates other cross platform issues in this 
> context. But by returning the JarFile reference, ( rather than propagating 
> the exception,)  it is then possible to invoke close on the JarFile, and 
> allow the explicit delete of the JarFile. Thus, mitigating a perceived 
> problem on Windows.
>
> As such, this is an issue in its own right, and as such it would appear that 
> there is merit in this fix.

Thanks for your comments! Would it be appropriate to move the CSR to
"Proposed" [1] at this point?

>
> [...]
>

[1] https://wiki.openjdk.java.net/display/csr/Main#Main-CSRWorkflows

--
-Alex

Re: RFR[8243507]: 'DatagramSocket constructors don’t always specify what happens when passed invalid parameters'

[mark sheppard]

Hi Daniel, Patrick,
I wonder is there an opportunity here to refine the constructor 
descriptions a
little, also

The wording associated with wildcard addressing includes

* an IP address chosen by the kernel.

which is not actually correct, and maybe it should be omitted from
the various constructor descriptions.

there is explicit reference to IPv4 INADDR_ANY (0.0.0.0) which may warrant
a rewording to more general wildcard address {@link 
InetAddress#isAnyLocalAddress}

I think the following wording was proposed offline
changing


* If the IP address is 0.0.0.0, the socket will be bound to the

* {@link InetAddress#isAnyLocalAddress wildcard} address,

* an IP address chosen by the kernel.


to


* If the IP address is a {@link InetAddress#isAnyLocalAddress wildcard} address

* (or is {@code null}), the socket will be bound to the wildcard address.

also changing


The socket will be bound to the

{@link InetAddress#isAnyLocalAddress wildcard} address,

an IP address chosen by the kernel.


to


The socket will be bound to the

{@link InetAddress#isAnyLocalAddress wildcard} address.


best regards
Mark




From: net-dev  on behalf of Daniel Fuchs 

Sent: Tuesday 28 April 2020 15:32
To: Patrick Concannon ; OpenJDK Network Dev list 

Subject: Re: RFR[8243507]: 'DatagramSocket constructors don’t always specify 
what happens when passed invalid parameters'

Hi Patrick,

Looks good to me.
I realize we haven't specified what happens if the `laddr` is null.
Are you planning to fix that in a separate changeset or tag it to
this one?

Maybe we should unify the description of the `port` parameter in
the two constructors:

  327  * @param  port port to use.
  357  * @param port local port to use

Maybe:

   * @param port local port to use in the bind operation.

Nit: It's customary to break line before :

  343  * address.  The local port must be
between 0 and

suggest:

  343  * address.
   * The local port must be

best regards,

-- daniel

On 28/04/2020 10:33, Patrick Concannon wrote:
> Hi,
>
> Could someone please review my fix for JDK-8243507 'DatagramSocket
> constructors don’t always specify what happens when passed invalid
> parameters'?
>
> Currently, the DatagramSocket constructor `DatagramSocket(SocketAddress
> bindaddr)` doesn't specify what happens if passed a SocketAddress
> subclass not supported by this socket.
> Also, there are two DatagramSocket constructors that accept a port
> number, but neither constructor specifies what happens when passed port
> 0, or a port which falls outside of the valid range of port numbers i.e
> between 0 and 65535 inclusive.
>
> This fix updates the spec for each these constructors to inform the user
> of what happens when passed an invalid argument. For the constructors
> that take a port, the spec will now specify that an
> IllegalArgumentException is thrown when passed a port outside of the
> acceptable range, or, if zero is passed, that the system will choose an
> appropriate port for them. For the constructor that takes a
> SocketAddress, an IllegalArgumentException will be thrown if an invalid
> SocketAddress subclass is passed as a parameter.
>
> bug: https://bugs.openjdk.java.net/browse/JDK-8243507
> csr: https://bugs.openjdk.java.net/browse/JDK-8243976
> webrev: http://cr.openjdk.java.net/~pconcannon/8243507/webrevs/webrev.00/
>
>
> Kind regards,
>
> Patrick
>

Re: RFR[8237890]: 'DatagramPacket::getSocketAddress doesn't specify what happens if address or port are not set'

[mark sheppard]

Hi Patrick et al.
   a trivial point and a javadoc question

trivial item in SendCheck test, there's a comment referring to pkt3 & 4 and 
invalid port -1.
This is no longer the case -- afaik it's now "can't send port 0 " in a 
SocketException ?

I note that there are some java doc changes and wonder is there an opportunity 
to make some further
amendments:

for getSocketAddress and setSocketAddress methods

the wording "of the remote host"  doesn't quite fit,
and could be replaced with   "of the application or service "

in general remote host could be replaced with host, as localhost provides  a 
valid IP address !!

references to machine could be replaced with host for consistency, also ?

regards
Mark


From: net-dev  on behalf of Patrick Concannon 

Sent: Thursday 16 April 2020 13:39
To: Chris Hegarty ; OpenJDK Network Dev list 

Subject: Re: RFR[8237890]: 'DatagramPacket::getSocketAddress doesn't specify 
what happens if address or port are not set'


Hi Chris,


Thanks for that.

I've added the new test cases as requested, and you can find them in the new 
webrev below.

http://cr.openjdk.java.net/~pconcannon/8237890/webrevs/webrev.02/


Kind regards,

Patrick

On 15/04/2020 16:54, Chris Hegarty wrote:

On 15 Apr 2020, at 16:40, Patrick Concannon 
 wrote:

...

webrev: http://cr.openjdk.java.net/~pconcannon/8237890/webrevs/webrev.01/



I added myself as reviewer on the CSR.

The changes mainly look good. Can you please add, or amend an existing, test 
for the newly specified default address and port values.

-Chris.

Re: RFR[8237890]: 'DatagramPacket::getSocketAddress doesn't specify what happens if address or port are not set'

[mark sheppard]

Hi Patrick,
if I understand the change correctly, you wish to eliminate the 
IllegalArgumentException and return an InetSocketAddress
based on the current set values for address and port. Because you have changed 
the default value of the port to 0, the getSocketAddress
will now return a SocketAddress with a wildcard address (assuming that address 
has not yet been set) and an ephemeral port, rather than the Exception caused 
by the port == -1.  Calling new InetSocketAddress(null, 0) will mean a wildcard 
address and an ephemeral port.
Port 0 is reserved for system selection of ephemeral port.  Thus, I think this 
change is now introducing some dubious semantics ?
Additionally if you were to call getSocketAddress multiple times you would get 
different non equal SocketAdress objects i.e. each one would have a different 
ephemeral port.

A possible approach is that an invocation of getSocketAddress should return 
null if the address or the port is not set, OR alternatively throw an 
IllegalStateException with either "port not set" or "address not set", as the 
DatagramPacket is in an unusable state for sending and the remote address is 
not set?

If we look at DatagramSocket::send() method , I suspect that there is another 
side effect to your change the check


if (packetPort < 0 || packetPort > 
0x)
throw new IllegalArgumentException("port out of range:" + 
packetPort);

will allow a DatagramPacket with port == 0  to continue its porcessing. But 
sending to port 0 is not allowed afaik
so should this be:
  if (packetPort <= 0 || packetPort > 0x)
  throw new IllegalArgumentException("port out of range: " + 
packetPort);

regards
Mark



From: net-dev  on behalf of Patrick Concannon 

Sent: Friday 10 April 2020 10:16
To: OpenJDK Network Dev list 
Subject: RFR[8237890]: 'DatagramPacket::getSocketAddress doesn't specify what 
happens if address or port are not set'


Hi,

Could someone please review my webrev and CSR for JDK-8237890 
'DatagramPacket::getSocketAddress doesn't specify what happens if address or 
port are not set' ?

DatagramPacket::getSocketAddress is misleading in that it can throw an 
IllegalArgumentException even though it doesn't take any arguments. This can 
occur if the port of a DatagramPacket is not set before a call to 
getSocketAddress is made.

In the recent fix 
JDK-8236940, additional 
checks were added to ensure DatagramPacket cannot be sent to port 0. Following 
on from this update, the current fix changes the default port of a 
DatagramPacket from -1 to 0. An IllegalArgumentException will therefore no 
longer be thrown when getSocketAddress with no port set. Instead, an 
InetSocketAddress representing any local address with port 0 is returned.

bug: https://bugs.openjdk.java.net/browse/JDK-8237890
csr: https://bugs.openjdk.java.net/browse/JDK-8242483
webrev: http://cr.openjdk.java.net/~pconcannon/8237890/webrevs/webrev.00/



Kind regards,

Patrick

Re: 8241995: Clarify InetSocketAddress::toString specification

[mark sheppard]

Hi Chris,
possible wording for your last paragraph:

To retrieve a string representation of the hostname, or in the
absence of a hostname, the string form of the address, use {@link
#getHostString()}, rather than parsing the toString string representation.

or

It is advised to use {@link#getHostString()} to obtain a hostname string, or
in the absence of a hostname, a string form of the address. Do not rely on
parsing the toString() return value to obtain a hostname string.

regards
Mark





From: net-dev  on behalf of Chris Hegarty 

Sent: Friday 3 April 2020 17:26
To: OpenJDK Network Dev list 
Subject: 8241995: Clarify InetSocketAddress::toString specification

As noted in a recent thread [1], the wording of InetSocketAddress::toString
could be improved a little, to avoid any potential confusion about how
and when "" is displayed. It was also suggested that a link
to getHostString could be added.

Below is an initial suggestion. It does not change the spec, just adds
some examples and guidance. I'm relatively happy with the examples, but
less so with the final advise paragraph. Suggestions welcome.


-  /**
-   * Constructs a string representation of this InetSocketAddress.
-   * This String is constructed by calling toString() on the InetAddress
-   * and concatenating the port number (with a colon). If the address
-   * is an IPv6 address, the IPv6 literal is enclosed in square brackets.
-   * If the address is {@linkplain #isUnresolved() unresolved},
-   * {@code } is displayed in place of the address literal.
-   *
-   * @return  a string representation of this object.
-   */
-  public String toString() { ... }
---
+  /**
+   * Constructs a string representation of this InetSocketAddress.
+   * This String is constructed by calling {@link InetAddress#toString()}
+   * on the InetAddress and concatenating the port number (with a colon).
+   *
+   *  If the address is an IPv6 address, the IPv6 literal is enclosed in
+   * square brackets, for example: {@code "localhost/[0:0:0:0:0:0:0:1]:80"}.
+   * If the address is {@linkplain #isUnresolved() unresolved},
+   * {@code } is displayed in place of the address literal, for
+   * example {@code "foo/:80"}.
+   *
+   *  To retrieve a string representation of the hostname, or the string
+   * form of the address if it doesn't have a hostname, use {@link
+   * #getHostString()}, rather than parsing the string representation.
+   *
+   * @return  a string representation of this object.
+   */
+  public String toString() { ... }

-Chris

[1]  https://mail.openjdk.java.net/pipermail/net-dev/2020-April/013747.html

Re: RFC: 8132359: JarURLConnection.getJarFile() resource leak when file is not found

[mark sheppard]

 p.s.

I had also meant to say in the response below, that the proposed getJaFile 
solution is
perfectly reasonable and would allow a recoverable strategy in an related
scenario where a URLConnection:: connect,  for the same type of entry URL,
throws a FNFException resulting in the same type of "resource leak".
But, in this case, with the proposed change the JarFile can be retrieved and 
closed.

regards
Mark




From: net-dev  on behalf of mark sheppard 

Sent: Wednesday 1 April 2020 16:03
To: Michael McMahon ; Alex Kashchenko 

Cc: Mark Sheppard ; net-dev@openjdk.java.net >> 
OpenJDK Network Dev list 
Subject: Re: RFC: 8132359: JarURLConnection.getJarFile() resource leak when 
file is not found

Hi Michael et al.,
just looking at the webrev ... the change in URLClassPath seems reasonable.
The change in JarURLConnection  has implications and would change the semantics 
of
the getJarFile method.

using the example accompanying this JBS item to demonstrate an issue caused by 
the caching mechanism
within the URLConnection framework, it should be noted that the jar URL is 
referencing
an non existent entry in the jar file entry. Thus some form of exception would 
be anticipated in this scenario.

With the proposed change, this will return a JarFile regardless of whether a 
referenced resource (URL) exists or not.

Examining  the call flows it can be observed that
the getJarFile invokes connect. This will retrieve a jar file via JarFileFactory
 and then, because the URL references an entry in the jar file, attempts
 to access the entry resulting in a  null return,  which generates a FNF 
exception to be thrown.

Also note that an explicit invocation of connect on the JarURLConnection 
instance will result in the FNFException.

the change in the JarURLConnection will return a jar file in this test scenario 
and not the FNF exception. Based on the methods
spec is that acceptable behaviour change?


public abstract JarFile getJarFile throws IOException

Return the JAR file for this connection.

Returns:
the JAR file for this connection. If the connection is a connection to an entry 
of a JAR file, the JAR file object is returned
Throws:
IOException<https://docs.oracle.com/javase/7/docs/api/java/io/IOException.html> 
- if an IOException occurs while trying to connect to the JAR file for this 
connection.
See Also:
URLConnection.connect()<https://docs.oracle.com/javase/7/docs/api/java/net/URLConnection.html#connect()>
and connect says
"Opens a communications link to the resource referenced by this URL, if such a 
connection has not already been established."

So should the getJarFile return be a reference to JarFile for an URL specifying 
an non existent entry  i.e. the resource doesn't exist?
Is the resource associated with a JarURLConnection the encapsulated JarFile? Or 
the target in the specified  in the URL ?

There are a series of similar behaviour anomalies in this area 
URL/URLConnection/JarURLConnection (on Windows) and in general they
can be attributed to the internal caching  mechanism, which is enabled OOTB, 
and its impact on the closing of file resource in Exception conditions 
scenarios.

Disabling caching for jar protocol , which will allow consistent and correct 
behaviour is one possibility.

As such an alternative or workaround  is to disable caching for the jar protocol
via the URLConnection::setDefaultUseCaches() on Windows where an application
may want to delete a jar file resource, for example:

URLConnection.setDefaultUseCaches("jar", false);

best regards
Mark


From: net-dev  on behalf of Michael McMahon 

Sent: Monday 16 March 2020 12:39
To: Alex Kashchenko 
Cc: net-dev@openjdk.java.net >> OpenJDK Network Dev list 

Subject: Re: RFC: 8132359: JarURLConnection.getJarFile() resource leak when 
file is not found

Hi Alex,

(and redirecting the thread to net-dev)

It looks like a straight forward solution and perhaps the compatibility test
could be challenged on the basis of reliance on implementation behavior
rather than the spec.

But, more important I think is the behavior change of the fix itself and
that will require
careful review which we can't commit to immediately. We will try and get
back to you
about it in a week or so.

Thanks,

Michael.

On 14/03/2020 00:08, Alex Kashchenko wrote:
> Hi,
>
> Based on these maillist threads:
>
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-March/065076.html
>
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-November/063643.html
>
>
> I am looking for comments and suggestions, whether the following
> change to JarURLConnection.getJarFile() behaviour may be acceptable:
>
> If, during connect() call, jarFile itself was created successfully,
> but access to (non-existent) jarEntry failed - return this jarFile to
> caller instead of throwing exception.
>
&g

Re: RFC: 8132359: JarURLConnection.getJarFile() resource leak when file is not found

[mark sheppard]

Hi Michael et al.,
just looking at the webrev ... the change in URLClassPath seems reasonable.
The change in JarURLConnection  has implications and would change the semantics 
of
the getJarFile method.

using the example accompanying this JBS item to demonstrate an issue caused by 
the caching mechanism
within the URLConnection framework, it should be noted that the jar URL is 
referencing
an non existent entry in the jar file entry. Thus some form of exception would 
be anticipated in this scenario.

With the proposed change, this will return a JarFile regardless of whether a 
referenced resource (URL) exists or not.

Examining  the call flows it can be observed that
the getJarFile invokes connect. This will retrieve a jar file via JarFileFactory
 and then, because the URL references an entry in the jar file, attempts
 to access the entry resulting in a  null return,  which generates a FNF 
exception to be thrown.

Also note that an explicit invocation of connect on the JarURLConnection 
instance will result in the FNFException.

the change in the JarURLConnection will return a jar file in this test scenario 
and not the FNF exception. Based on the methods
spec is that acceptable behaviour change?


public abstract JarFile getJarFile throws IOException

Return the JAR file for this connection.

Returns:
the JAR file for this connection. If the connection is a connection to an entry 
of a JAR file, the JAR file object is returned
Throws:
IOException 
- if an IOException occurs while trying to connect to the JAR file for this 
connection.
See Also:
URLConnection.connect()
and connect says
"Opens a communications link to the resource referenced by this URL, if such a 
connection has not already been established."

So should the getJarFile return be a reference to JarFile for an URL specifying 
an non existent entry  i.e. the resource doesn't exist?
Is the resource associated with a JarURLConnection the encapsulated JarFile? Or 
the target in the specified  in the URL ?

There are a series of similar behaviour anomalies in this area 
URL/URLConnection/JarURLConnection (on Windows) and in general they
can be attributed to the internal caching  mechanism, which is enabled OOTB, 
and its impact on the closing of file resource in Exception conditions 
scenarios.

Disabling caching for jar protocol , which will allow consistent and correct 
behaviour is one possibility.

As such an alternative or workaround  is to disable caching for the jar protocol
via the URLConnection::setDefaultUseCaches() on Windows where an application
may want to delete a jar file resource, for example:

URLConnection.setDefaultUseCaches("jar", false);

best regards
Mark


From: net-dev  on behalf of Michael McMahon 

Sent: Monday 16 March 2020 12:39
To: Alex Kashchenko 
Cc: net-dev@openjdk.java.net >> OpenJDK Network Dev list 

Subject: Re: RFC: 8132359: JarURLConnection.getJarFile() resource leak when 
file is not found

Hi Alex,

(and redirecting the thread to net-dev)

It looks like a straight forward solution and perhaps the compatibility test
could be challenged on the basis of reliance on implementation behavior
rather than the spec.

But, more important I think is the behavior change of the fix itself and
that will require
careful review which we can't commit to immediately. We will try and get
back to you
about it in a week or so.

Thanks,

Michael.

On 14/03/2020 00:08, Alex Kashchenko wrote:
> Hi,
>
> Based on these maillist threads:
>
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-March/065076.html
>
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-November/063643.html
>
>
> I am looking for comments and suggestions, whether the following
> change to JarURLConnection.getJarFile() behaviour may be acceptable:
>
> If, during connect() call, jarFile itself was created successfully,
> but access to (non-existent) jarEntry failed - return this jarFile to
> caller instead of throwing exception.
>
> bug: https://bugs.openjdk.java.net/browse/JDK-8132359
> webrev: http://cr.openjdk.java.net/~akasko/jdk/8132359/webrev.00/
>
> This change also allows to fix JDK-8232854 with the minimal change to
> URLClassPath (included with the patch).
>
> This change doesn't cause regression failures in java/net.
>
> This change causes one compatibility failure, when getManifest()
> doesn't throw expected IOException when URL points to non-existent
> class inside JAR.
>

Re: RFR [15] 8241988 DatagramSocket incorrectly caches the first set of socket options

[Mark Sheppard]

thanks Chris 


:+1 


regards 
Mark 

- Original Message - 
From: chris.hega...@oracle.com 
To: macanao...@hotmail.com, mark.shepp...@oracle.com, net-dev@openjdk.java.net 
Sent: Wednesday, 1 April, 2020 4:04:33 PM GMT +00:00 GMT Britain, Ireland, 
Portugal 
Subject: Re: RFR [15] 8241988 DatagramSocket incorrectly caches the first set 
of socket options 







On 1 Apr 2020, at 15:12, mark sheppard < macanao...@hotmail.com > wrote: 


Hi Chris, 
just looking at the supportedOptions method in the two classes 
DatagramSocket and MulticastSocket, if I haven't misread them, are 
they not the same? 
As such MulticastSocket could inherit that method from DatagramSocket ? 
(without the necessity to override) 

Good idea Mark. 


Updated webrev: 
https://cr.openjdk.java.net/~chegar/8241988/webrev.01/ 


-Chris. 

Re: RFR [15] 8241988 DatagramSocket incorrectly caches the first set of socket options

[mark sheppard]

Hi Chris,
  just looking at the supportedOptions method in the two classes
DatagramSocket and MulticastSocket, if I haven't misread them, are
they not the same?
As such MulticastSocket could inherit that method from DatagramSocket ?
(without the necessity to override)

regards
Mark


From: net-dev  on behalf of Chris Hegarty 

Sent: Wednesday 1 April 2020 13:29
To: OpenJDK Network Dev list 
Subject: RFR [15] 8241988 DatagramSocket incorrectly caches the first set of 
socket options

DatagramSocket incorrectly caches the first set of socket options that
it encounters (through an invocation of supportedOptions()),
subsequently returning that set of options regardless of what the
particular datagram socket impl actually supports.

Webrev:
  https://cr.openjdk.java.net/~chegar/8241988/webrev/
JIRA:
  https://bugs.openjdk.java.net/browse/JDK-8241988

This issue has been filed so as to extract the long(ish) standing issue
from the work being done to support JEP 373 [1]. The code changes in
this review are only slightly different to the changes being proposed in
the review for 373 [1], so can be merged will little effort.

-Chris.

[1] https://mail.openjdk.java.net/pipermail/net-dev/2020-April/013748.html

Re: RFR: 8232513: java/net/DatagramSocket/PortUnreachable.java still fails intermittently with BindException

[mark sheppard]

Hi Julia and Daniel,
   I ran the test as a standalone under  8 and 13 and got the following
on windows 10

$ java -version
java version "1.8.0_231"
Java(TM) SE Runtime Environment (build 1.8.0_231-b11)
Java HotSpot(TM) 64-Bit Server VM (build 25.231-b11, mixed mode)

$ java PortUnreachableTest
Attempting to recreate server socket with port: 64195
PortUnreachableTest.recreateServerSocket: returning socket == 
/XXX.XXX.0.211:64195 obtained at first attempt with no sleep
Exception in thread "main" java.net.SocketTimeoutException: Receive timed out
at 
java.net.DualStackPlainDatagramSocketImpl.socketReceiveOrPeekData(Native Method)
at java.net.DualStackPlainDatagramSocketImpl.receive0(Unknown Source)
at java.net.AbstractPlainDatagramSocketImpl.receive(Unknown Source)
at java.net.DatagramSocket.receive(Unknown Source)
at PortUnreachableTest.execute(PortUnreachableTest.java:136)
at PortUnreachableTest.main(PortUnreachableTest.java:151)

$ /cygdrive/c/JDK-INSTALLS/jdk-13/bin/java -version
openjdk version "13" 2019-09-17
OpenJDK Runtime Environment (build 13+33)
OpenJDK 64-Bit Server VM (build 13+33, mixed mode, sharing)

$ /cygdrive/c/JDK-INSTALLS/jdk-13/bin/java PortUnreachableTest
Attempting to recreate server socket with port: 60473
PortUnreachableTest.recreateServerSocket: returning socket == 
/XXX.XXX.0.211:60473 obtained at first attempt with no sleep
Exception in thread "main" java.net.SocketTimeoutException: Receive timed out
at 
java.base/java.net.DualStackPlainDatagramSocketImpl.socketReceiveOrPeekData(Native
 Method)
at 
java.base/java.net.DualStackPlainDatagramSocketImpl.receive0(DualStackPlainDatagramSocketImpl.java:124)
at 
java.base/java.net.AbstractPlainDatagramSocketImpl.receive(AbstractPlainDatagramSocketImpl.java:182)
at java.base/java.net.DatagramSocket.receive(DatagramSocket.java:815)
at PortUnreachableTest.execute(PortUnreachableTest.java:136)
at PortUnreachableTest.main(PortUnreachableTest.java:151)





The test has a potential problem,  that is the  socket close can
cause the attempted send from server socket to client not to occur.

the sock.send() can result in the application data being copied into the
kernel and queued for sending.  But the send may not take place immediately.
The data may not yet be sent when immediate close()
is executed causing the socket's buffers and data to be released without 
sending.

lines 24 etc
   DatagramSocket sock = recreateServerSocket(serverPort);
b = "Greetings from the server".getBytes();
packet = new DatagramPacket(b, b.length, addr, clientPort);
sock.send(packet);
sock.close();

placing a small delay before the close appears to allow the send to take place
   DatagramSocket sock = recreateServerSocket(serverPort);
b = "Greetings from the server".getBytes();
packet = new DatagramPacket(b, b.length, addr, clientPort);
sock.send(packet);
Thread.sleep(1000);
sock.close();


$ /cygdrive/c/JDK-INSTALLS/jdk-13/bin/java PortUnreachableTest
Attempting to recreate server socket with port: 58754
PortUnreachableTest.recreateServerSocket: returning socket == 
/XXX.XXX.0.211:58754 obtained at first attempt with no sleep
client received data packet Greetings from the server

msheppard@MSHEPPARD-PC /cygdrive/c/Users/Mark 
Sheppard/eclipse-workspace/AgentTester-FLIG-4026
$ java PortUnreachableTest
Attempting to recreate server socket with port: 55876
PortUnreachableTest.recreateServerSocket: returning socket == 
/XXX.XXX.0.211:55876 obtained at first attempt with no sleep
client received data packet Greetings from the server

might be worth considering that as an addition

regards
Mark


From: net-dev  on behalf of Daniel Fuchs 

Sent: Wednesday 4 December 2019 16:10
To: Julia Boes ; OpenJDK Network Dev list 

Subject: Re: RFR: 8232513: java/net/DatagramSocket/PortUnreachable.java still 
fails intermittently with BindException

Hi Julia,

Looks good to me - but is:
   31  * @library /test/lib
actually needed?

best regards,

-- daniel

On 04/12/2019 15:40, Julia Boes wrote:
> Hi,
>
> The test PortUnreachable closes a DatagramSocket and tries to rebind it
> to the same port, this causes the test to fail rarely with a
> BindException (I did 500 test runs and couldn't observe a failure). To
> decrease the likelihood of this to happen, the fix increases the number
> of bind retries and test repeats.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8232513
>
> Webrev: http://cr.openjdk.java.net/~jboes/webrevs/8232513/webrev.00/
>
> Regards,
>
> Julia
>

Re: RFR: 8233185: HttpServer.stop() blocks indefinitely when called on dispatch thread

[mark sheppard]

Hello,
   I may have added this to fix some other issue.
   it would seem reasonable that stop() would reap (join) the dispatcher thread 
that was launched in start()
What is not expected, based on the design of the Dispatcher,  that a stop() 
will be invoked from within its executing thread
(run method).
The dispatcher effectively dispatches events to a handler, and that appears to 
be done through an executor.
In the case of the test it would appear that a handler method is being called 
within the context of the dispatcher's run method ?

It is possible that there is a deeper issue with the dispatcher  and the run 
method needs
to be looked at, also?

In any case, it is shown to be possible to call the stop within the 
Dispatcher's thread, and the fix solves that issue 

regards
Mark


From: net-dev  on behalf of Vyom Tewari26 

Sent: Wednesday 27 November 2019 07:52
To: julia.b...@oracle.com 
Cc: net-dev@openjdk.java.net 
Subject: Re: RFR: 8233185: HttpServer.stop() blocks indefinitely when called on 
dispatch thread

Hi Julia,

thanks for looking into  this issue, it looks like regression, in JDK8  we  
don't do join() on "dispatcherThread" in stop().  We need to find out why we 
introduce the below code.


if (dispatcherThread != null) {
try {
dispatcherThread.join();
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
logger.log (Level.TRACE, "ServerImpl.stop: ", e);
}
}
#

Thanks,
Vyom

- Original message -
From: Julia Boes 
Sent by: "net-dev" 
To: OpenJDK Network Dev list 
Cc:
Subject: [EXTERNAL] RFR: 8233185: HttpServer.stop() blocks indefinitely when 
called on dispatch thread
Date: Tue, Nov 26, 2019 9:58 PM

Hi,

When HttpServer.stop(int delay) is called on the dispatcher thread of
the server, the call blocks indefinitely as the thread is waiting for
itself to die. The proposed fix in this case is to skip the join() and
let the thread return immediately.

Bug: https://bugs.openjdk.java.net/browse/JDK-8233185

Webrev: http://cr.openjdk.java.net/~jboes/webrevs/8233185/webrev.00/


Regards,

Julia

Re: RFR: 8231260: (dc) DatagramChannel::disconnect changes the port of the local address to 0 (lnx)

[mark sheppard]

OK  Alan, Daniel thanks for the feedback and consideration  … but what you say 
does lend an argument that
localAddress = null; rather than localAddress = isa;
is a reasonable value in the event of Net.bind throwing a BindException 

but, as you have said,  that may have other implications

leaving my dead donkey well and truly flogged!!

an influencing (rhetorical) question is
how soon is a port available for re-allocation/re-use after it has been 
released by the OS on Linux?
with  connected TCP sockets  it would expected to be the TTL setting … not sure 
about a connected UDP socket!
Would it be instantaneously available after its release by the kernel, or 
subject to TTL lifetime?

best regards
Mark



From: Daniel Fuchs 
Sent: Tuesday 8 October 2019 14:40
To: Alan Bateman ; mark sheppard 
; nio-dev 
Cc: OpenJDK Network Dev list 
Subject: Re: RFR: 8231260: (dc) DatagramChannel::disconnect changes the port of 
the local address to 0 (lnx)

On 08/10/2019 15:29, Alan Bateman wrote:
> On 08/10/2019 15:20, mark sheppard wrote:
>> :
>>
>> Q: is  localAddress.getPort() == 0   indicative that the
>> DatagramChannel is unbound ?
>>
> getLocalAddress() returns a SocketAddress when bound, it returns null
> when not bound. I don't think we should get too hung up on corner case
> that arises when the local port cannot be restored (by re-binding). The
> javadoc sets the expectations that the channel's socket is in an
> undefined state when disconnect fails.

+1 : you would be in uncharted territory and that's exactly why
we want to throw an exception when rebind fails and why we recommend
closing the channel when that happens.

-- daniel

>
> -Alan.

Re: RFR: 8231260: (dc) DatagramChannel::disconnect changes the port of the local address to 0 (lnx)

[mark sheppard]

Hi Daniel,
    thanks for the reply 

I also meant to add that as
on the ln 947   localAddress = isa;

so in the event of the Net.bind() throwing a BindException then the 
localAddress in
DatgramSocket instance will have a port == 0, indicating that it is unbound ?

if the localAddress.getPort() == 0   during the connect call,  would it be 
valid to also
add a check for this in conjunction to the null check ?

  // ensure that the socket is bound
  if (localAddress == null) {
  bindInternal(null);
  } else if (localAddress.getPort() == 0)
  InetSocketAddress rebindAddress = localAddress;
  localAddress = null;
  bind(rebindAddress);
  }

to generate a rebind to the original IP address but with a new ephemeral port ?

Q: is  localAddress.getPort() == 0   indicative that the DatagramChannel is 
unbound ?


best regards
Mark




From: Daniel Fuchs 

Sent: Tuesday 8 October 2019 09:16

To: mark sheppard ; Alan Bateman 
; nio-dev 

Cc: OpenJDK Network Dev list 

Subject: Re: RFR: 8231260: (dc) DatagramChannel::disconnect changes the port of 
the local address to 0 (lnx)

 


Hi Mark,



On 07/10/2019 16:09, mark sheppard wrote:

> Hi Daniel,

>     wrt impl note ... would some explanation on the esoteric reason for 

> a now possible BindException  be

> useful, also? 

[...]

> the output will show a java.net.BindException, which is sort of arcane 

> for a disconnect ?



There is such a thing as over specification for a dark system specific

corner case of the API.

The CSR [1] and release notes [2] should have enough documentation

without engraving such details in stone in the Java SE specification.



> If it is strongly recommended to close the DatagramChannel, why not 

> actually

> close the DatagramChannel in the event that the Net.bind throws a 

> BindException ?



That would be much more surprising and also have more backward

compatibility risks than the proposed fix.



> I presume that on linux, if the port is not an OS  chosen ephemeral 

> port,  i.e. it is a port from the registered port range

> that the reset of the port to 0 doesn't occur in a disconnect ?



Yes. Although AFAIK it's not the port range that matters, but whether

the port was originally chosen by the system or not.



> on the ln 947   localAddress = isa;

> 

> if this was  localAddress = null;  would that allow a connect to be 

> subsequently called and the DatagramChannel usable again,

> even when an BindException on the disconnect has occurred, as the  

> DatagramChannel would be rebound by the connect ?



As the API note say: it is strongly recommended to close the channel

if disconnect throws an IOException. Whether the channel is usable

or not. If you don't then you'd be depending on unspecified

behavior of the implementation.



> In any case, I think it would be informative to disclose some further 

> details on the possibility of a (re-)bind failure  in the disconnect !



The release note is there for that - it can be referred to if the

issue arises.



best regards,



-- daniel



[1] https://bugs.openjdk.java.net/browse/JDK-8231880

[2] https://bugs.openjdk.java.net/browse/JDK-8231881



> 

> best regards

> Mark

> 

> 

> 

> 

> *From:* net-dev  on behalf of Daniel 

> Fuchs 

> *Sent:* Monday 7 October 2019 11:34

> *To:* Alan Bateman ; nio-dev 

> 

> *Cc:* OpenJDK Network Dev list 

> *Subject:* Re: RFR: 8231260: (dc) DatagramChannel::disconnect changes 

> the port of the local address to 0 (lnx)

> Hi Alan,

> 

> Here is the new webrev - I believe I have addressed all your comments:

> 

> http://cr.openjdk.java.net/~dfuchs/webrev_8231260/webrev.01

> 

> best regards,

> 

> -- daniel

> 

> On 04/10/2019 14:55, Alan Bateman wrote:

>> On 04/10/2019 14:34, Daniel Fuchs wrote:

>>> :

>>>

>>> webrev:

>>> http://cr.openjdk.java.net/~dfuchs/webrev_8231260/webrev.00/

>>>

>> The apiNote looks okay. DatagramChannelImpl::disconnect looks okay but I 

>> assume "might not preserve" should be "does not preserve" (if you make 

>> that change then the "This is the expected ..." line isn't needed).

>> 

>> One suggestion is to rename the test to AddressesAfterDisconnect to make 

>> it a bit clearer that it tests the local/remote addresses after 

>> disconnect. I also assume we can change this to a TestNG test to avoid 

>> the need for the assertXXX methods.

>> 

>> -Alan.

> 

Re: RFR: 8231260: (dc) DatagramChannel::disconnect changes the port of the local address to 0 (lnx)

[mark sheppard]

Hi Daniel,
   wrt impl note ... would some explanation on the esoteric reason for a now 
possible BindException  be
useful, also? namely that on some platforms the socket is unbound  during the 
disconnect and requires a re-bind, which for
ephemeral ports allocated may result in a BindException.

try {
   statusUpdateDatagramChannel.disconnect();
} catch (IOException ioEx) {
   clientLogger.debug("Unexpected exception " + ioEx.getClass().getName();
   statusUpdateDatagramChannel.close();
   statusUpdateDatagramChannel = createStatusServiceDatagramChannel();
}
 ...

the output will show a java.net.BindException, which is sort of arcane for a 
disconnect ?

If it is strongly recommended to close the DatagramChannel, why not actually
close the DatagramChannel in the event that the Net.bind throws a BindException 
?

I presume that on linux, if the port is not an OS  chosen ephemeral port,  i.e. 
it is a port from the registered port range
that the reset of the port to 0 doesn't occur in a disconnect ?

on the ln 947   localAddress = isa;

if this was  localAddress = null;  would that allow a connect to be 
subsequently called and the DatagramChannel usable again,
even when an BindException on the disconnect has occurred, as the  
DatagramChannel would be rebound by the connect ?

In any case, I think it would be informative to disclose some further details 
on the possibility of a (re-)bind failure  in the disconnect !

best regards
Mark





From: net-dev  on behalf of Daniel Fuchs 

Sent: Monday 7 October 2019 11:34
To: Alan Bateman ; nio-dev 
Cc: OpenJDK Network Dev list 
Subject: Re: RFR: 8231260: (dc) DatagramChannel::disconnect changes the port of 
the local address to 0 (lnx)

Hi Alan,

Here is the new webrev - I believe I have addressed all your comments:

http://cr.openjdk.java.net/~dfuchs/webrev_8231260/webrev.01

best regards,

-- daniel

On 04/10/2019 14:55, Alan Bateman wrote:
> On 04/10/2019 14:34, Daniel Fuchs wrote:
>> :
>>
>> webrev:
>> http://cr.openjdk.java.net/~dfuchs/webrev_8231260/webrev.00/
>>
> The apiNote looks okay. DatagramChannelImpl::disconnect looks okay but I
> assume "might not preserve" should be "does not preserve" (if you make
> that change then the "This is the expected ..." line isn't needed).
>
> One suggestion is to rename the test to AddressesAfterDisconnect to make
> it a bit clearer that it tests the local/remote addresses after
> disconnect. I also assume we can change this to a TestNG test to avoid
> the need for the assertXXX methods.
>
> -Alan.

Re: (teststabilization) RFR: 8231506: Fix some instabilities in a few networking tests

[mark sheppard]

Hi Daniel,
   looks like an  interesting saga … will give that twirl on my dinky laptop

   wrt  why MulticastSocket  sets SO_REUSEADDR I reckon Chris or Alan are the 
authority on that.
Conjecture would be that typically multicast scenarios require multiple 
processes receiving
on the same address and port, with the prescribed idiom being wildcard IP 
address+ defined port.
Thus, requiring the SO_REUSEADDR  for multi instances on the same host,
and it was done as a de facto convenience.

The wildcard IP address idiom is somewhat perverse for multicast,
and seems to be rooted in restrictive multicast support available
on Windows.
Although, Michael McM did say Solaris exhibits restrictive behaviour also, 
inhibiting the binding to a multicast address.

regards
Mark

From: Daniel Fuchs 
Sent: Monday 30 September 2019 16:47
To: mark sheppard ; OpenJDK Network Dev list 

Subject: Re: (teststabilization) RFR: 8231506: Fix some instabilities in a few 
networking tests

Hi Mark,

On 30/09/2019 13:37, Daniel Fuchs wrote:
> On 30/09/2019 12:56, mark sheppard wrote:
>> the first uses the InetSocketAddress allocated and then the second
>> uses the same address, without bind  exception because of
>> the SO_REUSEADDR option being set,  even if the close had not
>> completed at that time.
>
> No. The second doesn't use the address. I agree that it might - if
> e.g. the system has run out of ephemeral ports, but I believe  it's
> highly unlikely.

I take it back. One of my test run exhibited this exact behavior:
The client's MulticastSocket was bound to the exact same address
than the server's MulticastSocket - and of course the test
failed (see below).

Then I played with that a bit by forcing the client to bind
to the same address than the server. On my machine, I could
observe that only the last bound socket received any traffic.
There seemed no way to make the server receive anything from
the client in that configuration.

So I revised the test logic: I added a Phaser to make sure
the client and server advanced in phase, and I also added
a loop to force the client to bind to a different address
(actually a different port) than the server if binding to
the ephemeral port returns the same port than the server.
(I could test that by forcing the first bind to use the
server port instead of port 0).

So here is now my new webrev:

http://cr.openjdk.java.net/~dfuchs/webrev_8231506/webrev.02/index.html

Only changes compared to webrev.01 are in UnreferencedMulticastSockets.

By any chance, do you remember why MulticastSocket sets SO_REUSEADDR ?

best regards,

-- daniel


=

   DatagramServer addr: localhost/127.0.0.1: 48429
   serverMulticastSocket:: fd: java.io.FileDescriptor@65dd3755, fd: 5,
cleanup: java.net.SocketCleanable@4ba89eb
socketImplClass: class java.net.PlainDatagramSocketImpl
socketImpl class name not matched: java.net.PlainDatagramSocketImpl !=
java.net.TwoStacksPlainDatagramSocketImpl
   client bound port: 48429
   clientMulticastSocket:: fd: java.io.FileDescriptor@179da90c, fd: 9,
cleanup: java.net.SocketCleanable@725335c5
socketImplClass: class java.net.PlainDatagramSocketImpl
socketImpl class name not matched: java.net.PlainDatagramSocketImpl !=
java.net.TwoStacksPlainDatagramSocketImpl
ping sent to: localhost/127.0.0.1:48429
echo received from: /127.0.0.1:48429
--System.err:(13/908)--
java.lang.AssertionError: incorrect data received: expected: 2, actual: 1
at 
UnreferencedMulticastSockets.main(UnreferencedMulticastSockets.java:151)
at
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native
Method)
at
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at
java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:564)
at
com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127)
at java.base/java.lang.Thread.run(Thread.java:830)

Re: (teststabilization) RFR: 8231506: Fix some instabilities in a few networking tests

[mark sheppard]

Hi Daniel,
  thanks for the reply ..


the client is on loopback with an ephemeral port

so in the following:

   try (MulticastSocket s = new MulticastSocket(clientAddress)) {
// no-op; close immediately
s.getLocalPort();   // no-op
} 

long fdCount0 = getFdCount();
listProcFD();

// start a server
Server svr = new Server();
Thread thr = new Thread(svr);
thr.start();

MulticastSocket client = new MulticastSocket(clientAddress);

the first uses the InetSocketAddress allocated and then the second uses the 
same address, without bind  exception because of
the SO_REUSEADDR option being set,  even if the close had not completed at that 
time.

The reference to asynchronous close in the try block above is at the OS/kernel 
level. The assumption is that a  synchronous
blocking call but that might not be the case within the OS (?) . This may leave 
two extant UDP sockets bound to the same
address at the time of the echo from the server, which may leave corresponding 
delivery to the UDP socket indeterminate.
Again, some extreme conjecture, but failures are intermittent and on heavily 
loaded system, I think, making concurrent activity
more indeterminate.

WRT server, the point here is that can you guarantee that in the following, the 
release of the server socket
is not without side effect.

   ss.send(p); // send back +1
// do NOT close but 'forget' the socket reference
ss = null;

the ss = null; releases the server socket and it is available for garbage 
collection, which theoretically could compete before any pending
i/o to the client has occurred in the kernel !! 

Thus, this ss = null could have a side effect such that on a heavily loaded 
system the send is pending (in the kernel) and the
release of the server socket object results in a close, due to garbage 
collection, before the send has completed.
Such a close could result in the pending send (i/o) being cancelled. Hence the 
client never receives its
echo response. 

you  previously used a CoundownLatch to synchronize  the client receive with 
the release of the server socket, to ensure that
the receive has occurred prior to the release of the server socket, thus 
ensuring the client receive happens before the
server socket is releases to eliminate the possibility of cancellation of 
pending i/o from the server.

 in the server

     CountDownLatch clientRxLatch = new CountDownLatch(1);

...

      ss.send();
      clientRxLatch.await();
      ss = null;


in the client thread

DatagramPacket p = new DatagramPacket(msg, msg.length, svr.getHost(), 
svr.getPort());
client.send(p);
svr.clientRxLatch.countDown();   
System.out.printf("ping sent to: %s:%d%n", svr.getHost(), 
svr.getPort());

perhaps worth considering ?

best regards
Mark










From: Daniel Fuchs 

Sent: Monday 30 September 2019 10:05

To: mark sheppard ; OpenJDK Network Dev list 


Subject: Re: (teststabilization) RFR: 8231506: Fix some instabilities in a few 
networking tests

 


Hi Mark,



On 30/09/2019 08:58, mark sheppard wrote:

> So does the second MulticastSocket need to use the same client unicast 

> address ?



The clientAddress is an InetSocketAddress with port 0. It is possible

that the second MulticastSocket will get the same port allocated to

it, because the first one has been closed - but it's probably unlikely.



>  Can it be assured that there is no asynchrony in the synthesized code for 
>the autoclose in the try with

> resources, both at the java level and within the OS kernel executing the 
> close?

> Would an explicit close of the first MulticastSocket add better determinacy 
> to the test execution?



try-with-resource will close the first socket just as in

try { } finally { } - there is no mystery here. So the answer

to the above is clearly no.



> n the DatagramSocket version, a level of synchronization between the server 
> thread and

> the main thread was added, would that be appropriate here again?



I don't think so all that matters is that the server DatagramSocket

is created before the datagram packet is sent.



best regards,



-- daniel

Re: (teststabilization) RFR: 8231506: Fix some instabilities in a few networking tests

[mark sheppard]

Hi Daniel,
   a couple of observations and few points to consider in the 
UnreferencedMulticastSockets
test.

similar to the DatagramSocket version but for this one MulticastSocket will 
automatically set
the so_reuseaddr option. This has implications when the second MulticastSocket 
is created.
Unlike the DatagramSocket socket version this test will use the same client 
address for both
MulticastSocket created. Thus there is a stronger dependency between the try 
with resourses
statement and the code that creates the second MulticastSocket for the send and 
receive.

So does the second MulticastSocket need to use the same client unicast address ?
There doesn't seem to be any multicast semantics in the test, other than 
creating a MukticastSocket,
which is_a DatagramSocket.

Can it be assured that there is no asynchrony in the synthesized code for the 
autoclose in the try with
resources, both at the java level and within the OS kernel executing the close?
Would an explicit close of the first MulticastSocket add better determinacy to 
the test execution?

It could happen in a heavily loaded system, that if the close is asynchronous 
(Windows ?) that when the
server has echo back the datagram there are still two extant sockets in the OS 
kernel and which socket receives
the packet is indeterminate - this is not a multicast send and receive but a 
unicast send and receive.
This could lead to intermittent client receive requests hanging (?)

Note also that the client socket "connect" to the server address, meaning that 
it sends and receive
datagrams from that peer address only.

In the DatagramSocket version, a level of synchronization between the server 
thread and
the main thread was added, would that be appropriate here again?

best regards
Mark

From: net-dev  on behalf of Daniel Fuchs 

Sent: Thursday 26 September 2019 14:16
To: OpenJDK Network Dev list 
Subject: (teststabilization) RFR: 8231506: Fix some instabilities in a few 
networking tests

Hi,

Please find below a patch for:

https://bugs.openjdk.java.net/browse/JDK-8231506
8231506: Fix some instabilities in a few networking tests

webrev:
http://cr.openjdk.java.net/~dfuchs/webrev_8231506/webrev.00/


There's one remaining test that still uses the wildcard
(SocketImplCombinations). It has been observed failing in the past,
and hopefully this change will fix it.

The UnreferencedMulticastSockets, which has also been observed
failing is also be updated with some more diagnosis.

Finally, the DigestEchoServer$TunnelingProxy class has also been
observed causing some instabilities in the httpclient tests that use it:
the proxy tunnel doesn't properly propagate errors from downstream to
upstream, and conversely, which sometimes leaves the reader waiting for
data that will never come. This change should fix that.

best regards,

-- daniel

Re: RFR [8193596]: java/net/DatagramPacket/ReuseBuf.java failed due to timeout

[mark sheppard]

Hi Alan, Daniel

a couple of observations on the assertion for test failure that you may wish to 
consider.
​
If there is a BindException for  the DatagramSocket instantiations​
then this would suggest that there is an operating system​ issue.
The sockets are being bound to an ephemeral port,  allocated by the OS, which 
would mean​
that the OS is choosing a port that it has  already allocated ! ​
​
It may be worth checking that the ephemeral port range in the test environments 
are appropriately configured,​
as per IANA recommendations.​
​
One potential extreme condition is that IFF there are many thousands of 
concurrent tests executing,​
there could be ephemeral port exhaustion ?
​
Another observation is that this is a UDP test, as such, it is unreliable in 
its outcome.​
That is to say, UDP sends are not guaranteed to be successful.​
This could be especially true in a very heavily loaded system, which may have 
some resource contention,​
such as available UDP buffer space. As such, there is no guarantee that any 
send will succeed.​
It may be an extreme exaggeration, but the OS may accept a message to send, 
copy from user space to kernel space,​
but because of some extreme exceptional conditions in the kernel, drops the 
message without notification.​
​
Another point to keep in mind is that the test is multi threaded, and again in 
heavily loaded system the scheduling of​
a thread may not be as prompt as expected ?

best regards
Mark


From: net-dev  on behalf of Daniel Fuchs 

Sent: Friday 23 August 2019 09:42
To: Alan Bateman ; Patrick Concannon 
; OpenJDK Network Dev list 

Subject: Re: RFR [8193596]: java/net/DatagramPacket/ReuseBuf.java failed due to 
timeout

Hi Alan,

On 23/08/2019 10:25, Alan Bateman wrote:
> On 23/08/2019 10:10, Patrick Concannon wrote:
>> Hi,
>>
>> Would it be possible to have my fix for JDK-8193596 reviewed?
>>
>> The test fails intermittently due to binding to wildcard addresses --
>> similar to those that have been fixed under the umbrella of
>> JDK-8222938 . This
>> fix changes the binding from using a wildcard address to localhost.
>>
> It's hard to see from the bug report if this interference due to binding
> to the wildcard address but the change looks okay to me.

Yes I agree - but it fails again then at least that's one thing we
won't have to put in question :-)

best regards,

-- daniel

Re: [teststabilization] RFR 8229348: java/net/DatagramSocket/UnreferencedDatagramSockets.java fails intermittently

[mark sheppard]

Hi Daniel,
   thanks for the reply 

regards
Mark


From: Daniel Fuchs 
Sent: Monday 12 August 2019 10:44
To: mark sheppard ; OpenJDK Network Dev list 

Subject: Re: [teststabilization] RFR 8229348: 
java/net/DatagramSocket/UnreferencedDatagramSockets.java fails intermittently

Hi Mark,

On 10/08/2019 21:14, mark sheppard wrote:
> It is suggested that the use of wildcard address (INADDR_ANY) is a
> contributing factor, but i'm​
> not sure that is a correct assumption.

That certainly did fix the intermittent failure we had in
the new httpclient tests - especially on solaris, back when we
were heavily hammering them prior to integration.

> Just to emphasize the reuse address option is not on the IP address but
> rather on the IP address and port ​
> combination, or at least that what is was meant to be. In the TCP
> context there are other restrictions, also.​

yes.
  ​
> Your change is to the "server socket", but the client is a symmetric
> equivalent, DatagramSocket on​
> wildcard and ephemeral port, so the echo send from the server could
> equally be sent astray!!​
> That is, if the wildcard addressing is an issue.​

Oh - darn. Thanks for noticing this! I should change the client too.

> Looking at the overall structure of the test, is it not possible, that
> the server socket has been​
> GCed, finalized and so closed, prior to the server's packet send having
> been completed within the OS, and so the​
> client hangs. ​

I doubt this would happen with UDP since the message should already
have been sent - but just in case I'm adding a countdown latch just
to eliminate the possibility.

Here is a new webrev:
webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8229348/webrev.01/

best regards,

-- daniel

> 8229348: java/net/DatagramSocket/UnreferencedDatagramSockets.java
>fails intermittently
> https://bugs.openjdk.java.net/browse/JDK-8229348

Re: RFR: 8219804 ava/net/MulticastSocket/Promiscuous.java fails intermittently due to NumberFormatException

[mark sheppard]

Hi Michael, Chris,
a brief note on the possibility of stray packets.​

For the test to receive data from external sources it would be necessary that 
the senders are​
using the same port ( as well as the mcast address) as your test (which is an 
ephemeral port).​
So the source of such stray data could be concurrently executing instances of 
the tests​
on other test machines when each test's ephemeral port coincidentally match !!​
​
However, the fact that you are repairing a NumberformatException would indicate 
that the source​
was not another instance of this test, as the test would receive a string 
representation of an integer.​
However,  Chris alludes to /test/java/nio/channels/DatagramChannel/Promiscuous, 
and​
this uses the same mcast address (pre your changes) as this test and its 
integer data would not be interpretable by this test, afaik.
​
So it is the source of possible conflict and a NumberFormatException. -- 
depending on the same ephemeral port allocation taking place.
​
Should the test check that multicast is enabled on the test host?​
​
the sender's address could always be output in the receive method ?​
​
as a side issue:​
Noting the idioms used in the test binding to wildcard address for the 
receiving socket​
so the test is open to receiving data on any address associated with the test 
host, that is ​
destined to the ephemeral port allocated. The ephemeral port limits receipt of 
stay data but​
doesn't eliminates it. The alternative is to bind to mcast address.​
​
Sender Datagram socket is unbound, so will send on the default interface (?)​
​
What's the expectation when the test executes in multihomed environment?​
​
This has been a problem for tests in the past (in my limited experience), 
especially the​
sending over the correct interface.​
​
Is the assumption that if the test host is multihomed, then the appropriate OS​
configuration for sending/receiving on the multicast exists, such that the send 
will​
go over an interface configured for your chosen mcast address?​
That is to say, the sender will send on the appropriate interface​
for the specified multicast address in a multihomed scenario.​
It's assumed the default interface is multicast enabled
 and that interface has a route specified.​
​
​
maybe a few things to consider.​
​
regards​
Mark


From: net-dev  on behalf of Michael McMahon 

Sent: Friday 21 June 2019 16:27
To: Chris Hegarty
Cc: OpenJDK Network Dev list
Subject: Re: RFR: 8219804 ava/net/MulticastSocket/Promiscuous.java fails 
intermittently due to NumberFormatException

Hi Chris,

On 21/06/2019, 12:32, Chris Hegarty wrote:
> Michael,
>
> On 21/06/2019 11:53, Michael McMahon wrote:
>> Small test case update. The test has failed a couple of times where
>> it appears
>> to be receiving input on a multicast socket which could not be
>> generated by the test case itself.
>> The test happens to use multicast groups that are assigned by IANA,
>> and globally routable.
>> So, it is conceivable that other entities are sending packets picked
>> up by the test.
>> The test also does not protect against other instances of itself
>> running on different hosts
>> at the same time, though that doesn't seem to be the cause of this
>> failure.
>> The change is to use non-routable multicast groups and to add some
>> hopefully unique data
>> to the test in case the test might be running on multiple hosts on
>> the same subnet simultaneously.
>>
>> http://cr.openjdk.java.net/~michaelm/8219804/webrev.1/index.html
>
> I think this is ok.
>
> With this change, the negative scenarios ( that are expected to
> timeout ), are susceptible to retrying when/if rogue packets are
> received ( I guess this is less likely now, since the groups are
> non-routable ). Would it be helpful to just print out the ignored
> packet / data ( in case of future reliability issues )?
>
Yeah, I'll add some logging for that eventuality.

> There is a nio test, java/nio/channels/DatagramChannel/Promiscuous.java
> that follows a similar pattern. Should it be updated in a similar way?
>
I notice that test uses a "reserved" multicast address, which applications
are not supposed to use. Maybe, routers won't forward those packets either.
I think I'd prefer to leave that test as it is, for now, especially
seeing as it
hasn't failed.

Updated at:
http://cr.openjdk.java.net/~michaelm/8219804/webrev.2/

Thanks,
Michael.

Re: RFR: 8223716: sun/net/www/http/HttpClient/MultiThreadTest.java should be more resilient to unexpected traffic

[mark sheppard]

Hi Daniel,
 thanks for the reply and clarifications
apologies for the distraction

regards
Mark


From: Daniel Fuchs 
Sent: Thursday 16 May 2019 09:19
To: mark sheppard; Chris Hegarty; OpenJDK Network Dev list
Subject: Re: RFR: 8223716: sun/net/www/http/HttpClient/MultiThreadTest.java 
should be more resilient to unexpected traffic

Hi Mark,

AFAIU the test wants to verify that connections are reused.
It does that by creating N (N=5) threads that will hammer
at the server concurrently.

Because there are N threads, then there can't be more than
N concurrent requests, and therefore there should not be
more than N connections and therefore N workers.

What we observe is that from time to time the server receives
N+1 connections. It happens very infrequently - but I've seen it
a couple of times.

When that happens - I usually that the 6th connection is created
much later in the game - and by that time one of the other connections
(usually worker 0) no longer seems to receive anything.

So for some reason one connection has been closed, and a
new one has been created to take up the load.
I have yet not clue why that happens, and JDK-8223783 has
been logged to followup on this.

In the mean time I modified the test to not fail but
print a warning when N+1 connections are created when N are
expected.

best regards,

-- daniel


On 16/05/2019 00:35, mark sheppard wrote:
> Hi Daniel,
>a little feedback on the test and some observations.
> was curious about this test, mainly that debug wasn't synchronized
> and expected to see interleaved output from clients and servers.
> So ran the test … had look at the output, which wasn't interleaved
> and totals all seemed to matched
>
>
> as I understand it the test outline is that
> create Server
> accept client connection
> create Worker for connection
> start worker
>
> create 5 clients
> create HttpUrlConnection to test server
> send 20 requests
>
> should see total 100 requests
> 20 from each client and 20 received by each worker
>
> On looking at the output a little closer things seemed a little off
> or my observations maybe wrong. Each of the workers i.e. the client
> handlers in the server didn't receive 20 request but a random number
> of requests. This seems to exhibit some of the pathology of the original
> bug ?
>
> Ran the test multiple times and a total of 100 requests received but
> randomly distributed across
> the workers
>
> To observe the client side output I amended test constructor to take an
> int clientId and used that
> in that clients debug output.
>
> FWIW attached in a file with analysis of a few test runs output.
>
> regards
> Mark

Re: RFR: 8223716: sun/net/www/http/HttpClient/MultiThreadTest.java should be more resilient to unexpected traffic

[mark sheppard]

Hi Daniel,
  a little feedback on the test and some observations.
was curious about this test, mainly that debug wasn't synchronized
and expected to see interleaved output from clients and servers.
So ran the test … had look at the output, which wasn't interleaved
and totals all seemed to matched


as I understand it the test outline is that
create Server
accept client connection
create Worker for connection
start worker

create 5 clients
create HttpUrlConnection to test server
send 20 requests

should see total 100 requests
20 from each client and 20 received by each worker

On looking at the output a little closer things seemed a little off
or my observations maybe wrong. Each of the workers i.e. the client
handlers in the server didn't receive 20 request but a random number
of requests. This seems to exhibit some of the pathology of the original bug ?

Ran the test multiple times and a total of 100 requests received but randomly 
distributed across
the workers

To observe the client side output I amended test constructor to take an  int 
clientId and used that
in that clients debug output.

FWIW attached in a file with analysis of a few test runs output.

regards
Mark


msheppard@MARKS-LAPTOP /cygdrive/c/Users/msheppard/eclipse-workspace/JDK-Tests
$ /cygdrive/c/Development/jdk-installs/jdk-12/bin/java 
MultiThreadedHttpUrlConnectionTest > MTHUCT-4.out​
​
msheppard@MARKS-LAPTOP /cygdrive/c/Users/msheppard/eclipse-workspace/JDK-Tests​
$ more MTHUCT-4.out​
server: calling accept.​
server: return accept.​
server: Started worker 0​
server: calling accept.​
server: return accept.​
worker 0: Read request from client (162 bytes).​
server: Started worker 1​
server: calling accept.​
server: return accept.​
worker 1: Read request from client (162 bytes).​
server: Started worker 2​
server: calling accept.​
server: return accept.​
server: Started worker 3​
server: calling accept.​
server: return accept.​
worker 2: Read request from client (162 bytes).​
server: Started worker 4​
server: calling accept.​
client 0: read 11 bytes​
client 3: read 11 bytes​
client 1: read 11 bytes​
worker 3: Read request from client (162 bytes).​
worker 0: Read request from client (162 bytes).​
worker 2: Read request from client (162 bytes).​
worker 1: Read request from client (162 bytes).​
worker 4: Read request from client (162 bytes).​
client 3: read 11 bytes​
client 4: read 11 bytes​
client 1: read 11 bytes​
client 0: read 11 bytes​
worker 2: Read request from client (162 bytes).​
worker 0: Read request from client (163 bytes).​
worker 1: Read request from client (162 bytes).​
worker 3: Read request from client (163 bytes).​
client 4: read 11 bytes​
client 1: read 11 bytes​
client 3: read 11 bytes​
worker 1: Read request from client (163 bytes).​
worker 0: Read request from client (163 bytes).​
client 0: read 11 bytes​
client 4: read 11 bytes​
client 1: read 11 bytes​
worker 3: Read request from client (163 bytes).​
worker 1: Read request from client (163 bytes).​
worker 2: Read request from client (163 bytes).​
worker 0: Read request from client (163 bytes).​
client 4: read 11 bytes​
client 3: read 11 bytes​
client 0: read 11 bytes​
client 1: read 11 bytes​
worker 1: Read request from client (163 bytes).​
worker 2: Read request from client (163 bytes).​
worker 3: Read request from client (163 bytes).​
worker 0: Read request from client (163 bytes).​
client 4: read 11 bytes​
client 3: read 11 bytes​
client 0: read 11 bytes​
worker 1: Read request from client (163 bytes).​
client 1: read 11 bytes​
worker 2: Read request from client (163 bytes).​
worker 3: Read request from client (163 bytes).​
client 4: read 11 bytes​
client 3: read 11 bytes​
worker 0: Read request from client (163 bytes).​
worker 1: Read request from client (163 bytes).​
client 0: read 11 bytes​
worker 2: Read request from client (163 bytes).​
client 1: read 11 bytes​
client 4: read 11 bytes​
client 3: read 11 bytes​
worker 3: Read request from client (163 bytes).​
worker 0: Read request from client (163 bytes).​
worker 1: Read request from client (163 bytes).​
worker 2: Read request from client (163 bytes).​
client 0: read 11 bytes​
client 1: read 11 bytes​
client 3: read 11 bytes​
client 4: read 11 bytes​
worker 3: Read request from client (163 bytes).​
worker 0: Read request from client (163 bytes).​
worker 2: Read request from client (163 bytes).​
worker 1: Read request from client (163 bytes).​
client 0: read 11 bytes​
client 3: read 11 bytes​
client 1: read 11 bytes​
client 4: read 11 bytes​
worker 0: Read request from client (163 bytes).​
worker 3: Read request from client (163 bytes).​
worker 2: Read request from client (163 bytes).​
worker 1: Read request from client (163 bytes).​
client 3: read 11 bytes​
client 4: read 11 bytes​
client 0: read 11 bytes​
client 1: read 11 bytes​
worker 0: Read request from client (163 bytes).​
worker 3: Read request from client (163 bytes).​
worker 2: Read request from client (163 bytes).​
worker 1: Read request from client 

Re: [ipv6] RFR: 8223737: HostsFileNameService doesn't handle IPv6 literal addresses correctly

[mark sheppard]

Hi Arthur,
yes all good    thanks

regards
Mark


From: Arthur Eubanks 
Sent: Wednesday 15 May 2019 02:06
To: mark sheppard
Cc: Chris Hegarty; OpenJDK Network Dev list
Subject: Re: [ipv6] RFR: 8223737: HostsFileNameService doesn't handle IPv6 
literal addresses correctly



On Tue, May 14, 2019 at 4:14 AM mark sheppard 
mailto:macanao...@hotmail.com>> wrote:
Hi Chris,
yes understood,  thanks for that ... all good  :+1  ... just wanted to 
highlight how the current mechanism worked
and
that on  a Windows lapper there is a potential anomaly with the standard 
platform names service with the reverse lookup scenario.

regards
Mark
Mark, is the +1 for a review for this change or for Mark's explanation?

Re: [ipv6] RFR: 8223737: HostsFileNameService doesn't handle IPv6 literal addresses correctly

[mark sheppard]

Hi Arthur, Chris,
   just a note in passing, as you are well set on the changes, which is all  
good -- needs must, as they say.

The current implementation is an emulation of the gethostbyname and 
gethostbyaddr lookup on /etc/hosts.
The reverse lookup  issue is also solved by adding an additional entry in the 
hosts file
0:0:0:0:0:0:0:1 ip6-localhost ip6-loopback. Your reverse lookup will pass.

$ /cygdrive/c/Development/jdk-installs/jdk-12/bin/java 
InternalNameServiceWithHostsFileTest
  InetAddress == ip6-localhost/0:0:0:0:0:0:0:1​
retrieved address == [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1] equal to 
expected address == [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1]​
testReverseLookup: host addr ::1 : expected host name ip6-localhost​
testReverseLookup: host addr 0:0:0:0:0:0:0:1 : expected host name ip6-localhost​
testReverseLookup: host addr :::::::0001 : expected 
host name ip6-localhost​

That was the design -- the hosts file would contain whatever set of mapping are 
required for a particular context.
So the string comparison was sufficient.

IF your reverse lookup tests are run against the PlatformFormNameService,
which is the fall through to the native OS calls gethostbyname, getaddrinfo etc.
then you get the same set of failure afaik.

on windows using jdk12 test output shown below
so you could say there is some degree of consistent behaviour between the two 
services!!

regards
Mark



msheppard@MARKS-LAPTOP /cygdrive/c/Users/msheppard/eclipse-workspace/JDK-Tests
$ /cygdrive/c/Development/jdk-installs/jdk-12/bin/java  
-Djava.net.preferIPv6Addresses=true PlatformNameServiceTest​
  InetAddress == localhost/0:0:0:0:0:0:0:1​
retrieved address == [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1] equal to 
expected address == [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1]​
testReverseLookup: host addr ::1 : expected host name localhost​
Exception in thread "main" java.lang.RuntimeException: reverse lookup of "::1" 
is "0:0:0:0:0:0:0:1", should be "localhost"​
​
at 
PlatformNameServiceTest.testReverseLookup(PlatformNameServiceTest.java:64)​
at PlatformNameServiceTest.main(PlatformNameServiceTest.java:25)​
​
msheppard@MARKS-LAPTOP /cygdrive/c/Users/msheppard/eclipse-workspace/JDK-Tests​
$ /cygdrive/c/Development/jdk-installs/jdk-12/bin/java  
-Djava.net.preferIPv6Addresses=true PlatformNameServiceTest​
  InetAddress == localhost/0:0:0:0:0:0:0:1​
retrieved address == [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1] equal to 
expected address == [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1]​
testReverseLookup: host addr 0:0:0:0:0:0:0:1 : expected host name localhost​
Exception in thread "main" java.lang.RuntimeException: reverse lookup of 
"0:0:0:0:0:0:0:1" is "0:0:0:0:0:0:0:1", should be "localhost"​
​
at 
PlatformNameServiceTest.testReverseLookup(PlatformNameServiceTest.java:64)​
at PlatformNameServiceTest.main(PlatformNameServiceTest.java:26)​
​
msheppard@MARKS-LAPTOP /cygdrive/c/Users/msheppard/eclipse-workspace/JDK-Tests​
$ /cygdrive/c/Development/jdk-installs/jdk-12/bin/java  
-Djava.net.preferIPv6Addresses=true PlatformNameServiceTest​
  InetAddress == localhost/0:0:0:0:0:0:0:1​
retrieved address == [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1] equal to 
expected address == [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1]​
testReverseLookup: host addr :::::::0001 : expected 
host name localhost​
Exception in thread "main" java.lang.RuntimeException: reverse lookup of 
":::::::0001" is "0:0:0:0:0:0:0:1", should be 
"localhost"​
​
at 
PlatformNameServiceTest.testReverseLookup(PlatformNameServiceTest.java:64)​
at PlatformNameServiceTest.main(PlatformNameServiceTest.java:27)​



From: net-dev  on behalf of Arthur Eubanks 

Sent: Monday 13 May 2019 17:59
To: Chris Hegarty
Cc: OpenJDK Network Dev list
Subject: Re: [ipv6] RFR: 8223737: HostsFileNameService doesn't handle IPv6 
literal addresses correctly



From: Chris Hegarty mailto:chris.hega...@oracle.com>>
Date: Mon, May 13, 2019 at 9:33 AM
To: Arthur Eubanks, OpenJDK Network Dev list

Arthur,

On 11/05/2019 01:17, Arthur Eubanks wrote:
> Bug: https://bugs.openjdk.java.net/browse/JDK-8223737
> Webrev: http://cr.openjdk.java.net/~aeubanks/8223737/webrev.00/index.html
>
> HostsFileNameService doesn't handle IPv6 literal addresses correctly.
> For example, ::1 and 0:0:0:0:0:0:0:1 should yield the same output.
>
> Rather than comparing address strings, compare the address byte arrays.

Comparison of byte arrays is much better. The changes look good.

Trivially, can I ask you to please include a number of other reverse
lookup's to fill out the test a little. Here's what I came up with:

 testReverseLookup("10.2.3.4", "testHost.testDomain");

 // ::1 and 

Re: [ipv6] RFR: 8223532: Don't try creating IPv4 sockets in NetworkInterface.c if IPv4 is not supported

[mark sheppard]

Hi Arthur et al.
with these changes and the increased separation of IPv4 and IPv6 in the 
call flows, does
it make sense to preclude the retrieval of IPv6 interfaces, if there is an 
error in the IPv4 processing in the
enumInterfaces function ?  Or  that an  error retrieving IPv6 interfaces 
precludes returning IPv4 interfaces already retrieved?

Is it worth considering  the alternative to record the error and continue the 
attempt to retrieve IPv6 interfaces.
If that succeeds clear the exception and return the IPv6 interfaces, and vice 
verse. An error in the IPv6 retrieval will
preclude the return of the IPv4 interfaces already retrieved.

A potential cause of failure in opening a socket is FD exhaustion at the moment 
of invocation (e.g. a constrained environment with open files 64 !!)
which may have dissipated, if the call flow proceeds to retrieve the IPv6, 
while at the same time another thread has released an fd with file close or 
socket close.

If not  then it may be worth  considering that any Exception thrown should 
elaborate the cause a bit more -- indicating the root cause is either IPv4 or 
IPv6 based,  as per openSocketWithFallback.

has the semantics of the openSocketWithFallback now changed ?
was it not that an AF_INET6 socket creation was attempted if the AF_INET (IPv4) 
socket creation failed ?
the predicate was on EPROTONOSUPPORT rather than EAFNOSUPPORT which is 
encaspulated in ipv4_available()
Now it's if ipv4 not available then try ipv6.
I know Chris made the point about treating them the same ... maybe so.
But at a pedantic level, the function is creating datagram socke heret, while 
IPv4_supported created a stream socket!!

anyway main point -- should a failure to retrieve IPv4 interfaces preclude IPv6 
interface retrieval and vice verse.


regards
Mark



From: net-dev  on behalf of Arthur Eubanks 

Sent: Wednesday 8 May 2019 17:33
To: Alan Bateman
Cc: OpenJDK Network Dev list
Subject: Re: [ipv6] RFR: 8223532: Don't try creating IPv4 sockets in 
NetworkInterface.c if IPv4 is not supported

Reverted changes in net_util.c.
Also, webrev.00 would create an IPv6 socket even if creating the IPv4 socket 
was successful. Fixed. (My very first revision had this same issue, which I 
thought I had fixed before sending it out. Tricky if statements have been 
cleaned up to make this less likely to happen again in the future.)

http://cr.openjdk.java.net/~aeubanks/8223532/webrev.01/

From: Alan Bateman mailto:alan.bate...@oracle.com>>
Date: Wed, May 8, 2019 at 7:58 AM
To: Chris Hegarty, Arthur Eubanks, OpenJDK Network Dev list



On 08/05/2019 12:51, Chris Hegarty wrote:
>
> While the vast majority of libnet.so is devoted to socket related
> implementation, not all is. There are a small number of low-level pieces
> of functionality that can be used with support for either IPv4 or IPv6
> being present. The NIO implementation also uses some shared common
> functionality from libnet.so. It seems overly restricting to disallow
> libnet.so from loading if neither IPv4 or IPv6 are present.
>
I agree as an innocent reference to a type in java.net might 
resulting
in libnet being loaded as a side effect. We also have the issue that the
file system implementation is in libnio so libnet will be loaded there
too (although shouldn't trigger its JNI OnLoad to be run).

-Alan

Re: RFR: 8223145: [teststabilization] Replace wildcard address with loopback or local host in test - part 1

[mark sheppard]

​
Hi Daniel,​
​
interesting set of changes -​
But could it be the case that, for some tests, you might change the operational 
semantics of a test, when​
this applying this change. For example, in the case of GetLocalAddress.​
The original is to use a wild card for the server, and a directed address for 
the ​
client connect. Thus the server is listening INADDR_ANY or the IPv6 equivalent, 
accepts​
a connection, then a check is made to see that socket created is bound to the 
same address​
as that specified in the clients connect request.​
​
If the server socket is  bound to a specific address then the socket created by 
the accept​
will (always)  have the same address as that of initial server socket 
(listener).​
​
As the tests are run with 3 different java.net property settings, it is worth 
asking if the tests behaves as expected​
for the case when neither java.net property is set.​
AFAIK, on windows it shows some ambiguity and difference to the default setting 
described in​
[1]. The default run returns IPV4 addresses, creates AF_INTET sockets and binds 
to IPv4 addresses.​
This is possibly at odds with the default property settings are described in 
the docs.​
​
java.net.preferIPv4Stack (default: false)​
If IPv6 is available on the operating system the underlying native socket will 
be, by default,​
an IPv6 socket which lets applications connect to, and accept connections from, 
both IPv4 and IPv6 hosts. ​
This implies that the first test run, i.e. default property settings, should be 
an AF_INET6 socket as in new ServerSocket​
should be bound to the IPv6 unspecified address.​
​
While​
java.net.preferIPv6Addresses (default: false)​
When dealing with a host which has both IPv4 and IPv6 addresses, ​
and if IPv6 is available on the operating system, the default ​
behavior is to prefer using IPv4 addresses over IPv6 ones.​
​
This implies that the InetAddress.getLocalHost() should in the default case​
return an IPv4 address.​
​
The client socket connect with IPv4 address to IPv6 wild card resulting in an 
IPv4 mapped​
binding in the socket created in the accept. This would mean the first run 
should fail.​
​
In any case a test using a server socket listening on a wildcard (INADDR_ANY) 
address has possibly​
slightly different semantics than a server socket listening on a specific host 
address.​
​
regards
Mark
​
​
​
[1] 
https://docs.oracle.com/javase/8/docs/api/java/net/doc-files/net-properties.html



From: net-dev  on behalf of Daniel Fuchs 

Sent: Tuesday 30 April 2019 11:16
To: OpenJDK Network Dev list
Subject: RFR: 8223145: [teststabilization] Replace wildcard address with 
loopback or local host in test - part 1

Hi,

Please find below a patch for:

8223145: [teststabilization] Replace wildcard address with loopback
  or local host in test - part 1
https://bugs.openjdk.java.net/browse/JDK-8223145

http://cr.openjdk.java.net/~dfuchs/webrev_8223145/webrev.00/

This is the first in a series of patches that will try to
address intermittent failures which are sometime observed
when tests use the wildcard address to bind their test servers.

These changes are IpV6-only friendly.

best regards,

-- daniel

Re: [ipv6] Re: [RFR]: 8222562: IPv6 only systems fail on setsockopt(IPV6_V6ONLY, 0)

[mark sheppard]

an observation on IPv4_supported and IPV6_supported for your consideration



 both, IPv4_support and IPv6_support use socket creation in the appropriate AF 
domain as a
a verification of support, but the v6 version also checks that there is a set of
address binding or ipv6 address configuration by accessing /proc/net/if_inet6

afaik there is no equivalent /proc/net/if_inet or if_inet4, thus the ipv4 
version more
liberal in its deliberation.

as such, it is possible that for IPv4 the support is available but not yet 
configured,
but that has no impact on availability.
For IPv6 that the support is enabled but not yet configured and that will 
impact the
availability evaluation.

Implicit in this is the /proc/net/if_inet6 considered absolutely necessary ?

this has at times been the cause of spurious intermittent test failures if my 
memory is correct.

Another consideration is that if an application starts when IPv6 has yet to be 
configured properly,
it will most likely lead to the application requiring a restart after the IPv6 
to access IPv6 functionality




regards
Mark


From: net-dev  on behalf of Daniel Fuchs 

Sent: Wednesday 24 April 2019 10:40
To: Arthur Eubanks; Chris Hegarty
Cc: OpenJDK Network Dev list
Subject: Re: [ipv6] Re: [RFR]: 8222562: IPv6 only systems fail on 
setsockopt(IPV6_V6ONLY, 0)

Hi Arthur,

The jdk.changeset file in your webrev looks completely wrong.
http://cr.openjdk.java.net/~aeubanks/ipv6setsockopt/webrev.02/jdk.changeset
I assume it's just garbage and we should ignore it.
Can you confirm?

WRT to the individual files changes listed at
http://cr.openjdk.java.net/~aeubanks/ipv6setsockopt/webrev.02/index.html

In unix/native/libnet/net_util_md.c ipv4_supported():

I was wondering if we should be more selective about
what kind of errors we should consider meaning that
IPv4 is not available. But given that this method
executes in OnLoad and that there is a precedent with
ipv6_supported() I believe that what you have is fine.

The rest looks very reasonable to me.

Could you try to fix the jdk.changeset in the webrev so that we
could try to import and test your proposed changes?

best regards,

-- daniel

On 23/04/2019 16:54, Arthur Eubanks wrote:
> I believe that `DONT_ENABLE_IPV4`is not strictly required. Maybe just
> drop it?  Is it an initial attempt to support an IPv6-only JDK build?
> If so, then maybe we separate out that requirement.
>
> It was in the IPv6 code right below so I just copied it in case we
> wanted an IPv6-only build at some point. But yes that can be in a
> separate change. Removed.
> New webrev:
> http://cr.openjdk.java.net/~aeubanks/ipv6setsockopt/webrev.02/index.html

Re: [RFR]: 8222562: IPv6 only systems fail on setsockopt(IPV6_V6ONLY, 0)

[mark sheppard]

a couple of points and observations

* What is the platform's semantics when both java.net.preferIPv4Stack and 
java.net.preferIPv6Address are set simultaneously.
* Should the EAFNOSUPPORT check be more pervasive within the native code?
* IPv4 and IPv6 available doesn't mean that the stacks have been appropriately 
configured at the OS network level
* It would seems worthwhile making access to the platforms perspective of stack 
duality directly available at the API level
* It would seem worthwhile making the NetworkConfiguration abstraction 
available as util class in the API.
* Is it possible at the OS level to dynamically turn on and off IPv6 support 
and IPv4 support?
Thus a more dynamic test on on OS config is needed, rather than the onload 
config setting.
* It may be useful to define the set of use case scenarios for the different 
possible IPv4 IPv6 combinations and
  descibe the expected behaviour

1. What is the platform's semantics when both java.net.preferIPv4Stack and 
java.net.preferIPv6Address are set simultaneously.

The java.net.preferIPv4Stack precludes IPv6 usage,
[ … However, in the case an application would rather use IPv4 only sockets,
then this property can be set to true. ]. While java.net.preferIPv6Addresses,
does not exclude IPv4,  [ … This property can be set to true to change that
preference and use IPv6 addresses over IPv4 ones where possible].


what should be the effect if an applications sets these properties during the 
course of its execution?
afaik currently they are only accessed during startup and
initialization and set the operational behaviour immutably thereafter.

Thus, in IPv6_available is set on load.
IPv6_available = IPv6_supported() & (!preferIPv4Stack);

This doesn't take into account that an application might change it operational 
semantics dynamically ?


2. Should the EAFNOSUPPORT check be more pervasive within the native code?

Is it not the case now, with the additional IPv6 support considerations, that 
the socket system call returns
need to be augmented with an errno == EAFNOSUPPORT, also. Also as per the 
socket man pages description,
the other errors are somewhat spurious
and not indicative of no support, with exception of EPROTONOSUPPORT.  So the 
generalised < 0 check
should be refined with the additional  check for errno ==  EAFNOSUPPORT .
The other errno could considered exceptions and thrown as such. OR add 
EAFNOSUPPORT to handleSocketError ?


so in IPv6_supported()
if (fd < 0 && errno == EAFNOSUPPORT) {
return JNI_FALSE;
} else {
// some approriate handling the others errors
 // are they indicative non availability or a transient
 // OS config error
 // could be run out of file descriptions!
handleSocketError(...);
}

OR treat the other socket errors as spurious and proceed optimistically

if (fd < 0 && errno == EAFNOSUPPORT) {
return JNI_FALSE;
}



The java.net.preferIPv4Stack precludes IPv6 usage, so that is determined on load

Thus in the case of  Java_sun_nio_ch_Net_socket0

if the socket creation fails, should it be necessary to determine the semantics 
of
the failure rather than just exiting … maybe the OS ran out of file descriptors 
!
which domain was the socket creation using ... it could be IPv6
was supported at startup and then got reconfigured and is not supported when 
the socket call
is being made so then an attempted should be made for IPv4  AF_INET  or IPv4 
was configured
and now IPv6 instead  (some puppet or chef jobby decided to reconfigure the 
host network configuration)

if (fd < 0 && errno != EAFNOSUPPORT) {
// handle the error
} else {
// check the preferences and attempt a socket  in AF_INET domain
}


3. IPv4 and IPv6 available doesn't mean that the stacks have been appropriately 
configured at the OS network level
4. It would seem worthwhile making the NetworkConfiguration abstraction 
available as util class in the API.

Also worth noting that availability of either IPv6 and IPv4 doesn't guarantee 
that they
have been configured in the operating system at the network interface level. As 
such, while
sockets may be created their proper functioning may be impaired.
Thus, the usefulness of the utility class NetworkConfiguration
for determining how various interfaces are configured.
As a convenience at the application level it may be worth considering promoting
the NetworkConfiguration  from the test environment to the java.net API as a 
utility class.

Similarly IPSupport abstract provides some convient functionality.
An alternative implementation to this abstraction could  be to make available 
the IPv6_available IPv4_available via
IPv6supported and  IPv4supported methods  as part of an IPSupport API utility 
class.


5. Is it possible at the OS level to dynamically turn on and off IPv6 support 
and IPv4 support?

Thus a more dynamic test on on OS config is needed, rather than the onload 
config setting.
the global IPv6_available is set once on load, afaik - which may 

Re: RFR[10]: JDK-8023649 - java.net.NetworkInterface.getInterfaceAddresses() call flow clean up

[Mark Sheppard]

Hi Vyom,
   thanks for the feedback ...  interesting question and at a more 
general level is it safe to release a local reference
if there is a pending exception ...looking at the logic in getByName0 
function, it would appear that it is possible
to release the name_utf reference before the createNetworkInterface call 
and avoid any potential pending exception



regards
Mark

On 13/09/2017 04:16, vyom tewari wrote:

Hi Mark,

Thanks for doing this, i see that "createNetworkInterface" method is 
getting called from multiple places in NetworkInterface.c there is 
pending JNI Exception at line 695 in NetworkInterface.c. I am not sure 
if it is safe to call "(*env)->ReleaseStringUTFChars" even if there is 
pending JNI Exception.


Thanks,

Vyom


On Tuesday 12 September 2017 10:50 PM, Mark Sheppard wrote:

Hi,
    please oblige and review the follows changes:
http://cr.openjdk.java.net/~msheppar/8023649/webrev/

for the issue:
https://bugs.openjdk.java.net/browse/JDK-8023649

This is performed under the auspices of reliability, robustness and 
stability.
* As such, a number of error checks are amended to free malloc-ed 
memory,
* to add consistency to the code, additional ExceptionCheck have been 
added as a post  SetObjectArrayElement invocation check,
* A long standing issue reporting that 
NetworkInterface::getInterfacesAddresses can throw an NPE has been 
addressed.
The context for such a failure is that there is an IPv4 only 
configuration and that configuration is incorrect in its setting.
This may lead to the bindings array being allocated, but expected 
InterfaceAddress objects not allocated
and set in the array  so the bindings array implicitly contains 
null references.


In NetworkInterface.c  the function
createNetworkInterface
performs a check on an address mask which may lead to skipping the 
code block handling the InterfaceAddress allocation.


regards
Mark

Re: RFR 8085875/10, java/net/DatagramSocket/PortUnreachable.java fails intermittently: Address already in use

[Mark Sheppard]

Hi Christoph,
   looking at the logic of the test, it is seen that a "server socket" 
is created in the execute method, and is then closed,
and a bunch of datagrams sent to it.  Then in the serverSend method an 
attempt is made to (re-)create a DatagramSocket using the
same port as the original "server socket", so recreate would seem to 
convey an appropriate semantics.
WRT an upper limit for retryCount, that's a reasonable point, but 
probably a "static final int" rather than just static field.


regards
Mark

On 07/09/2017 10:40, Langer, Christoph wrote:

Hi Felix,

this looks good in general.

I would, however, suggest to rename the method 'recreateServerSocket' into ' 
createServerSocket' as the former name suggests that something which existed 
once was recreated. But in this case the socket is simply created with a few 
retries when exceptions occur.

I'd also prefer if the retry count could be stored in a static field to allow 
for easy modification, instead of literally coding it to the value of 5 in the 
method.

And a minor thing: in line 63 there's a blank in between "recreateServerSocket" and 
"(int serverPort)" which should be removed.

Best regards
Christoph


-Original Message-
From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of
Felix Yang
Sent: Mittwoch, 6. September 2017 10:06
To: net-dev@openjdk.java.net; Mark Sheppard
<mark.shepp...@oracle.com>
Subject: RFR 8085875/10, java/net/DatagramSocket/PortUnreachable.java
fails intermittently: Address already in use

Hi all,

      please review a patch to add retries to avoid possible port
conflicts during re-binding.

Bug:

      https://bugs.openjdk.java.net/browse/JDK-8085875

Webrev:

      http://cr.openjdk.java.net/~xiaofeya/8085875/webrev.00/

Thanks,

Felix

Re: RFR 8134989/10, java/net/MulticastSocket/TestInterfaces.java failed due to unexpected IP address

[Mark Sheppard]

Hi
  it's  worth noting that there is diagnostic output for each of the 
test scenarios, and that for a failure scenario
the details of the network interface associated with the failure is 
displayed.


a typical failure scenario for this test is where a host has multiple 
network interfaces configured with an IPv4 address
and an unassigned IPv6 address. The unassigned IPv6 address causes 
problems in the MulticastSocket::getInterface call flow

which in turn uses NetworkInterface::getByInetAddress

this has been rectified in JDK10, afaik, by 
https://bugs.openjdk.java.net/browse/JDK-8179559


also, for the above configuration scenario, i.e. multiple network 
interfaces configured with unassigned IPv6 address, the
findInterfacewithDupAddress may not show duplicate addresses as the 
scope id will be different.
Would the ifconfig command allow two different network interfaces to be 
configured with the same IP address?
Not sure, but would expect not. Also would expect (IPv6) the scope id 
and index to be different.


NetworkConfiguration.printSystemConfiguration() for a complete 
diagnostic is a possible alternative.


regards
Mark

On 01/09/2017 14:41, Roger Riggs wrote:

Hi Felix,

Looks ok; though could be simpler to just print all the addresses of 
all NIs.


findIntefacesWithDupAddress could use streams more effectively (if its 
worth the time to rewrite).


The inetAddresses() method on NetworkInterface produces a stream of 
InetAddress which could

be filtered by "ia" and use anyMatch:.

Maybe something like:

List nis = NetworkInterface.networkInterfaces()
    .filter(i -> !i.equals(ni))
    .filter(i -> i.inetAddresses().anyMatch(n -> 
n.equals(ia)))

    .collect(Collectors.toList());

$.02, Roger

On 9/1/2017 3:47 AM, Felix Yang wrote:

Hi there,

    please review a test patch for isolating a network configuration 
issue, which led to TestInterfaces failing from time to time.


Bug:

    https://bugs.openjdk.java.net/browse/JDK-8134989

Webrev:

    http://cr.openjdk.java.net/~xiaofeya/8134989/webrev.00/

Thanks,

Felix

Re: RFR : 8182672: Java 8u121 on Linux intermittently returns null for MAC address

[Mark Sheppard]

a minor observation:
perhaps a slight modification to the test to allow adaptation to jdk8  
(the genesis of the reported problem)
replacing the NetworkInterface.networkInterfaces() with static method 
which encapsulates the Stream

creation

public static Stream 
getNetworkInterfacesAsStream() throws Exception {

//return NetworkInterface.networkInterfaces();

return 
Collections.list(NetworkInterface.getNetworkInterfaces()).stream();

}


public static void main(String[] args) throws Exception {
//List toTest = 
NetworkInterface.networkInterfaces()

List toTest = getNetworkInterfacesAsStream()
.filter(hasHardwareAddress)
.collect(Collectors.toList());

nonetheless execution of the upgraded test produces neat  compact 
diagnostic output with

the exceptions displayed neatly - looks good

regards
Mark

On 23/06/2017 14:47, Seán Coffey wrote:
Thanks all. There were a few shouts from Chris in the office when he 
saw my Enumeration code ;)


I'm running this through our test system again and will push if I get 
green results.


Regards,
Sean.

On 23/06/17 14:40, Langer, Christoph wrote:

Hi,

looks like a great piece of modern concurrent Java coding :) Well 
done! +1


Best regards
Christoph


-Original Message-
From: Chris Hegarty [mailto:chris.hega...@oracle.com]
Sent: Freitag, 23. Juni 2017 15:28
To: Seán Coffey ; Langer, Christoph

Cc: net-dev 
Subject: Re: RFR : 8182672: Java 8u121 on Linux intermittently 
returns null for

MAC address



On 23/06/17 10:56, Seán Coffey wrote:

Thanks to Christoph, Vyom and Mark for the reviews.

I've improved the testcase as per feedback. Hope this meets all 
requests :


http://cr.openjdk.java.net/~coffeys/webrev.8182672.jdk10.v2/webrev/

The change looks good.

Sean and I did a live coding session and arrived at the following
version of the test.

-Chris.

/*
   * Copyright (c) 2017, Oracle and/or its affiliates. All rights 
reserved.

   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
   *
   * This code is free software; you can redistribute it and/or 
modify it
   * under the terms of the GNU General Public License version 2 
only, as

   * published by the Free Software Foundation.
   *
   * This code is distributed in the hope that it will be useful, 
but WITHOUT

   * ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or
   * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
License
   * version 2 for more details (a copy is included in the LICENSE 
file that

   * accompanied this code).
   *
   * You should have received a copy of the GNU General Public License
version
   * 2 along with this work; if not, write to the Free Software 
Foundation,

   * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
   *
   * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 
94065

USA
   * or visit www.oracle.com if you need additional information or 
have any

   * questions.
   */

/*
   * @test
   * @bug 8182672
   * @summary Java 8u121 on Linux intermittently returns null for MAC
address
   */

import java.net.NetworkInterface;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.Phaser;
import java.util.function.Predicate;
import java.util.stream.Collectors;

public class GetMacAddress implements Callable {
  static final int NUM_THREADS = 5;
  static final int NUM_ITERS = 100;
  static volatile boolean failed; // false

  final String threadName;
  final NetworkInterface ni;
  final Phaser startingGate;

  public GetMacAddress(NetworkInterface ni, String name, Phaser 
phaser) {

  this.ni = ni;
  this.threadName = name;
  this.startingGate = phaser;
  }

  @Override
  public Exception call() {
  int count = 0;
  startingGate.arriveAndAwaitAdvance();
  try {
  for (int i = 0; i < NUM_ITERS; i++) {
  ni.getMTU();
  byte[] addr = ni.getHardwareAddress();
  if (addr == null) {
  System.out.println(threadName + ". mac id is 
null");

  failed = true;
  }
  count = count + 1;
  if (count % 100 == 0) {
  System.out.println(threadName + ". count is " 
+ count);

  }
  }
  } catch (Exception ex) {
  System.out.println(threadName + ". Not expecting
exception:" + ex.getMessage());
  failed = true;
  return ex;
  }
  return null;
  }

  static final Predicate hasHardwareAddress = 
ni -> {

  try {
  

Re: RFR : 8182672: Java 8u121 on Linux intermittently returns null for MAC address

[Mark Sheppard]

Hi Sean,
  main change looks fine - it sorts the reported problem.

ran your test against jdk8 152 and a recent jdk9 builds to verify that 
your test failure occurs.

The failure is seen, together with an additional (intermittent) exception:

java.net.SocketException: No such device (ioctl(SIOCGIFHWADDR) failed)
at java.base/java.net.NetworkInterface.getMacAddr0(Native Method)
at 
java.base/java.net.NetworkInterface.getHardwareAddress(NetworkInterface.java:504)
at 
NetworkInterfaceGetMacAddressThreadedTest.run(NetworkInterfaceGetMacAddressThreadedTest.java:59)
at 
java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1161)
at 
java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)

at java.base/java.lang.Thread.run(Thread.java:844)

The exception looks a bit confusing when mingled with test output. But this
doesn't occur in the fix version.

The exception is most likely a consequence of the non thread safe 
characteristic of the the current Linux
getMacAddress - what is actually in the ifr variable, when multiple 
threads are modifying it, when the  ioctl is called?

(who knows, who can tell, is this heaven, is this hell!!)

picked up you fix build and the test passes  fine,  as does the original 
test case reporting the issue.


also from a consistency perspective, the test is modifying two shared 
static variable

without synchronization. Maybe these should be volatile, at least?

regards
Mark

On 22/06/2017 17:29, Seán Coffey wrote:
JDK 10 fix required to correct a race issue in NetworkInterface. I 
don't believe the ifreq struct needs to be static in any case. New 
auto unit testcase also. I propose to skip this fix for JDK 9 and fix 
in an update release for that family. I also plan to port this to 
jdk8u-dev.


https://bugs.openjdk.java.net/browse/JDK-8182672
webrev : http://cr.openjdk.java.net/~coffeys/webrev.8182672.jdk10/webrev/

regards,
Sean.

Re: RFR: 8179559 Solaris MulticastSocket issues

[Mark Sheppard]

If we look at the failure scenario then it seen that
with multiple network interfaces having IPv6 and IPv4 configurations,
where the IPv6 part is not fully configured and is not UP, but is
RUNNING

wrt the change in MulticastSocket, is there not a deeper issue here?
that is,  in the MulticastSocket.getInterface()  method  the invocation

InetAddress ia = (InetAddress) 
getImpl().getOption(SocketOptions.IP_MULTICAST_IF)


doesn't return the same address as that which was set by the
MulticastSocket.setInterface()

in the problematic configurations the IPv6 interface is not UP, but it 
is RUNNING, MULTICAST, and
the address is unspecified. It would be expected that the OS would 
return the address configured in the

previous setsockopt?
At least it would be assumed that the OS syscall would be using the 
state of the interface

as criteria to avoid returning the unspecified IPv6 address in
preference to the UP and RUNNING and selected IPv4 address?
It seems the OS preference is for IPv6 regardless of the state on the 
interface.

Is there an issue to be raised with Solaris OS ?


Additionally, wrt  the try block incorporating the  address search by
iterating  over the network interfaces, the catch clause will return ia
That is, the address retrieved with via the previous getOption invocation.
Should this not return null or even re-throw the Exception?
returning an Address in this case seems questionable.

with respect to the change in MulticastSocket/Test.java
should the address checks be considered as OS agnostic?

a number of multicast socket tests exclude the network interface 
unspecified address

and the loopback address for consideration.

regards
Mark


On 05/05/2017 11:02, Chris Hegarty wrote:

On 5 May 2017, at 10:23, Michael McMahon  wrote:

Could I get the following changed reviewed please?

It's a fix for the two Solaris Multicast socket tests that fail always.

http://cr.openjdk.java.net/~michaelm/8179559/webrev.1/

This looks ok to me Michael. Good to have this loong standing
issue finally addressed. Thank you.

-Chris.

Re: RFR: JDK-7155591 - [macosx] regression test java/net/MulticastSocket/SetOutgoingIf.java fail

[Mark Sheppard]

thanks Chris
wrt NetworkConfiguration, yes I considered using it, but distilled the 
change to

the small change shown

regards
Mark

On 05/05/2017 09:30, Chris Hegarty wrote:

On 4 May 2017, at 21:56, Mark Sheppard <mark.shepp...@oracle.com> wrote:

Hi,
please oblige and review the following change
http://cr.openjdk.java.net/~msheppar/7155591/webrev/

This is fine Mark. Alternatively, you could use the NetworkConfiguration
probe, but I’m just happy to see this test become more stable.

-Chris.

Re: RFR: JDK-7155591 - [macosx] regression test java/net/MulticastSocket/SetOutgoingIf.java fail

[Mark Sheppard]

Thanks Brian


On 04/05/2017 22:13, Brian Burkhalter wrote:

Hi Mark,

Not exactly my area but I did look over similar code that Chris wrote for [1] 
and your code here looks OK (aside from the more recent copyright year).

Thanks,

Brian

[1] https://bugs.openjdk.java.net/browse/JDK-8158270

On May 4, 2017, at 1:56 PM, Mark Sheppard <mark.shepp...@oracle.com> wrote:


Hi,
please oblige and review the following change
http://cr.openjdk.java.net/~msheppar/7155591/webrev/

to address the issue raised in
https://bugs.openjdk.java.net/browse/JDK-7155591

this identifies that the awdl interface causes issues for the SetOutgoingIf 
multicast socket test, and
the proposed solution is exclude it from test scenarios. As such, the change
adds a check to ignore this network interface during interface selection.

regards
Mark

RFR: JDK-7155591 - [macosx] regression test java/net/MulticastSocket/SetOutgoingIf.java fail

[Mark Sheppard]

Hi,
please oblige and review the following change
http://cr.openjdk.java.net/~msheppar/7155591/webrev/

to address the issue raised in
https://bugs.openjdk.java.net/browse/JDK-7155591

this identifies that the awdl interface causes issues for the 
SetOutgoingIf multicast socket test, and

the proposed solution is exclude it from test scenarios. As such, the change
adds a check to ignore this network interface during interface selection.

regards
Mark

Re: RFR: JDK-8179512 - Typo in HttpURLConnection documentation

[Mark Sheppard]

thanks Chris

On 02/05/2017 14:53, Chris Hegarty wrote:

Mark,

The change looks ok to me.

-Chris.

On 02/05/17 14:51, Mark Sheppard wrote:

Hi
   please oblige and review the minor change, to javadoc of
HttpURLConnection, below

which addresses the punctuation correction requested
in
https://bugs.openjdk.java.net/browse/JDK-8179512

regards
Mark

bash-4.1$ hg diff
src/java.base/share/classes/java/net/HttpURLConnection.java
diff -r e91c7b2a4481
src/java.base/share/classes/java/net/HttpURLConnection.java
--- a/src/java.base/share/classes/java/net/HttpURLConnection.java Fri
Apr 28 12:22:53 2017 -0700
+++ b/src/java.base/share/classes/java/net/HttpURLConnection.java Tue
May 02 14:42:36 2017 +0100
@@ -54,7 +54,7 @@
  * Security permissions
  * 
  * If a security manager is installed, and if a method is called which
results in an
- * attempt to open a connection, the caller must possess either:-
+ * attempt to open a connection, the caller must possess either:
  * a "connect" {@link SocketPermission} to the host/port
combination of the
  * destination URL or
  * a {@link URLPermission} that permits this request.

Re: RFR: JDK-8175325 - NetworkInterface.getInterfaceAddresses throws NPE when no addresses

[Mark Sheppard]

Chris, Martin,
   thanks for the feedback  I'll remove the initialization from the 
constructor


regards
Mark

On 07/03/2017 15:17, Chris Hegarty wrote:

Mark,


On 6 Mar 2017, at 23:21, Mark Sheppard <mark.shepp...@oracle.com> wrote:

tha's true from the Java side. I didn't exhaustively check if is possible that 
the bindings could
be returned uninitialized or null from native code - I don't think it is 
possible, but I added the
null check, just in case.

I agree with Martin’s comment. The null check should be sufficient.
…


There is a side issue here, relating to the synthesis of a NetworkInterface for 
a MulticastSocket
bound to a wildcard address. This is somewhat dubious semantics, and would seem 
to be worthy of review
at some stage in the future.

Yes, this should be looked at in some more detail in the future.

-Chris.

Re: RFR: JDK-8175325 - NetworkInterface.getInterfaceAddresses throws NPE when no addresses

[Mark Sheppard]

tha's true from the Java side. I didn't exhaustively check if is 
possible that the bindings could
be returned uninitialized or null from native code - I don't think it is 
possible, but I added the

null check, just in case.

regards
Mark

On 06/03/2017 22:16, Martin Buchholz wrote:

It would not have both the never-null property and the check for null.

I would probably just leave bindings null in the constructor and check 
for null whenever reading bindings.


On Mon, Mar 6, 2017 at 2:00 PM, Mark Sheppard 
<mark.shepp...@oracle.com <mailto:mark.shepp...@oracle.com>> wrote:


Hi,
   please oblige and review the change
http://cr.openjdk.java.net/~msheppar/8175325/webrev/
<http://cr.openjdk.java.net/%7Emsheppar/8175325/webrev/>

for the issue raised in
https://bugs.openjdk.java.net/browse/JDK-8175325
<https://bugs.openjdk.java.net/browse/JDK-8175325>

the scenario is that a MulticastSocket, bound to a wildcard
address, which has yet to have its NetworkInterface
set, will return a synthesized NetworkInterface for a
getNetworkInterface method call. The newly constructed
has no InterfaceAddress bindings instantiated, resulting in a NPE
when getInterfaceAddresses is invoked.
The fix initializes the bindings member variable with an empty
array, as per suggestion in the bug, and also,
for completeness, places a null check in the getInterfaceAddresses
method.

There is a side issue here, relating to the synthesis of a
NetworkInterface for a MulticastSocket
bound to a wildcard address. This is somewhat dubious semantics,
and would seem to be worthy of review
at some stage in the future.

regards
Mark

RFR: JDK-8175325 - NetworkInterface.getInterfaceAddresses throws NPE when no addresses

[Mark Sheppard]

Hi,
   please oblige and review the change
http://cr.openjdk.java.net/~msheppar/8175325/webrev/

for the issue raised in
https://bugs.openjdk.java.net/browse/JDK-8175325

the scenario is that a MulticastSocket, bound to a wildcard address, 
which has yet to have its NetworkInterface
set, will return a synthesized NetworkInterface for a 
getNetworkInterface method call. The newly constructed
has no InterfaceAddress bindings instantiated, resulting in a NPE when 
getInterfaceAddresses is invoked.
The fix initializes the bindings member variable with an empty array, as 
per suggestion in the bug, and also,

for completeness, places a null check in the getInterfaceAddresses method.

There is a side issue here, relating to the synthesis of a 
NetworkInterface for a MulticastSocket
bound to a wildcard address. This is somewhat dubious semantics, and 
would seem to be worthy of review

at some stage in the future.

regards
Mark

RFR: JDK-8164815 - 3 JCK NetworkInterface tests fail on Raspberry Pi

[Mark Sheppard]

Hi,
   please oblige and review the change
http://cr.openjdk.java.net/~msheppar/8164815/webrev/src/java.base/share/classes/java/net/NetworkInterface.java.sdiff.html

to address the issue raised in
https://bugs.openjdk.java.net/browse/JDK-8164815

It was found during testing that, when a system does not have at least one
configured network interface, the Enumeration is instantiated using a 
null value,

which results in an unexpected NPE, when it is used.

this change, adds a specific check of the return value from the getAll() 
native method, and throws

a SocketException should a null value have been returned.

regards
Mark



diff -r 4407b0525631 
src/java.base/share/classes/java/net/NetworkInterface.java
--- a/src/java.base/share/classes/java/net/NetworkInterface.java Tue Nov 
08 16:54:28 2016 -0800
+++ b/src/java.base/share/classes/java/net/NetworkInterface.java Thu Nov 
10 14:43:45 2016 +

@@ -335,15 +335,19 @@
  * {@link #getInetAddresses()} to obtain all IP addresses for this 
node

  *
  * @return an Enumeration of NetworkInterfaces found on this machine
- * @exception  SocketException  if an I/O error occurs.
+ * @exception  SocketException  if an I/O error occurs,
+ * or if the System does not have at least one configured
+ * network interface.
  * @see #networkInterfaces()
  */
 public static Enumeration getNetworkInterfaces()
 throws SocketException {
 NetworkInterface[] netifs = getAll();
-assert netifs != null && netifs.length > 0;
-
+if (netifs != null && netifs.length > 0) {
 return enumerationFromArray(netifs);
+} else {
+throw new SocketException("Platform Configuration problem, 
no network interfaces configured");

+}
 }

 /**
@@ -361,15 +365,19 @@
  * }
  *
  * @return a Stream of NetworkInterfaces found on this machine
- * @exception  SocketException  if an I/O error occurs.
+ * @exception  SocketException  if an I/O error occurs,
+ * or if the System does not have at least one configured
+ * network interface.
  * @since 9
  */
 public static Stream networkInterfaces()
 throws SocketException {
 NetworkInterface[] netifs = getAll();
-assert netifs != null && netifs.length > 0;
-
+if (netifs != null && netifs.length > 0) {
 return streamFromArray(netifs);
+}  else {
+throw new SocketException("Platform Configuration problem, 
no network interfaces configured");

+}
 }

 private static  Enumeration enumerationFromArray(T[] a) {

Re: RFR - 8166747: Add invalid network / computer name cases to isReachable known failure switch

[Mark Sheppard]

Hi Rob,
changes look reasonable ...
perhaps align the two additions below the existing ERROR_XXX set, all 
neat and tidy :-)


regards
Mark

On 27/09/2016 00:09, Rob McKenna wrote:

Hi folks,

Looking for a review of this simple addition to Inet4AddressImpl.c on Windows. 
As per the bug report:

In the ping4() call in Inet4AddressImpl.c on Windows there is a switch 
statement containing failure codes for IcmpSendEcho which correspond to well 
known and expected failures for this call when a host is not reachable. In 
these cases ping4() simply returns false as opposed to throwing an exception.

Prior releases of the JDK would return false when using the tcp ping method 
where we currently throw an exception with the ERROR_INVALID_COMPUTERNAME 
(Windows error code 1210) or ERROR_INVALID_NETNAME (1214) errors. We should add 
these cases to the switch statement for compatibility purposes.

http://cr.openjdk.java.net/~robm/8166747/webrev.01/

-Rob

Re: RFR - 8159410: InetAddress.isReachable returns true for non existing IP addresses

[Mark Sheppard]

it is good that you added  the additional error code, "cover all bases", 
as they say.
In any case your exception handling will inform if  something has been 
missed, should it occur.

So at the risk of triggering another MS curiosity, the changes look fine

regards
Mark

On 21/09/2016 19:45, Rob McKenna wrote:

The link would be handy:

http://cr.openjdk.java.net/~robm/8159410/webrev.02/

-Rob

On 21/09/16 07:44, Rob McKenna wrote:

I've updated the webrev here with the copyright year (thanks Christoph) and 
extra error codes. I overlooked the codes from the old implementation of 
tcp_ping4 above this code. These are winsock error codes which I would expect 
IcmpSendEcho to use, but in our testing it actually returned the system error 
codes in at least one situation:

https://msdn.microsoft.com/en-gb/library/windows/desktop/ms740668%28v=vs.85%29.aspx
https://msdn.microsoft.com/en-us/library/windows/desktop/ms681383%28v=vs.85%29.aspx

-Rob

On 21/09/16 06:32, Seán Coffey wrote:

spotted an interesting blog on the MSDN timeout issue here :
https://www.frameflow.com/ping-utility-flaw-in-windows-api-creating-false-timeouts/

Regards,
Sean.

On 21/09/16 17:42, Mark Sheppard wrote:

the IcmpSendEcho series of calls come with some idiosyncrasies in that
there is a minimum timeout that they can handle
think it is about 1000msecs. isReachable can specify a finer grained
timeout hence the need for timeout check

regards
Mark

On 21/09/2016 17:18, Vyom Tewari wrote:

Hi Rob,

Do you really think this extra check is required ?

if (pEchoReply->Status == IP_SUCCESS
+ && (int)pEchoReply->RoundTripTime <= timeout) I did not found any
doc(MSDN) which explains this. I think in case of IP_SUCCESS
"RoundTripTime" is always less than timeout. I think similar changes is
required in Inet6Address.c as well ? Thanks, Vyom

On Wednesday 21 September 2016 08:46 PM, Rob McKenna wrote:

Hi folks,

I'd like to fix a bug caused by an incorrect assumption. The IcmpSendEcho* 
calls can actually return a similar set of errors regardless of whether the 
call itself failed or succeeded. This change checks that both the call and the 
ping were successful. In addition to that it takes a number of common failure 
causes into account before deciding to throw an exception.

http://cr.openjdk.java.net/~robm/8159410/webrev.01/

-Rob

Re: RFR - 8159410: InetAddress.isReachable returns true for non existing IP addresses

[Mark Sheppard]

the IcmpSendEcho series of calls come with some idiosyncrasies in that 
there is a minimum timeout that they can handle
think it is about 1000msecs. isReachable can specify a finer grained 
timeout hence the need for timeout check


regards
Mark

On 21/09/2016 17:18, Vyom Tewari wrote:


Hi Rob,

Do you really think this extra check is required ?

if (pEchoReply->Status == IP_SUCCESS
+ && (int)pEchoReply->RoundTripTime <= timeout) I did not found any 
doc(MSDN) which explains this. I think in case of IP_SUCCESS 
"RoundTripTime" is always less than timeout. I think similar changes 
is required in Inet6Address.c as well ? Thanks, Vyom


On Wednesday 21 September 2016 08:46 PM, Rob McKenna wrote:

Hi folks,

I'd like to fix a bug caused by an incorrect assumption. The IcmpSendEcho* 
calls can actually return a similar set of errors regardless of whether the 
call itself failed or succeeded. This change checks that both the call and the 
ping were successful. In addition to that it takes a number of common failure 
causes into account before deciding to throw an exception.

http://cr.openjdk.java.net/~robm/8159410/webrev.01/

-Rob

Re: RFR - 8159410: InetAddress.isReachable returns true for non existing IP addresses

[Mark Sheppard]

Hi Rob,
 this looks good ...

do you think there is any need to replicate these changes in 
Inet6AddressImpl.c ? (or leave it alone and don't trouble trouble 
until trouble troubles you :-)


regards
Mark

regards
Mark
On 21/09/2016 16:16, Rob McKenna wrote:

Hi folks,

I'd like to fix a bug caused by an incorrect assumption. The IcmpSendEcho* 
calls can actually return a similar set of errors regardless of whether the 
call itself failed or succeeded. This change checks that both the call and the 
ping were successful. In addition to that it takes a number of common failure 
causes into account before deciding to throw an exception.

http://cr.openjdk.java.net/~robm/8159410/webrev.01/

-Rob

Re: RFR 8153674: Expected SocketException not thrown when calling bind() with setReuseAddress(false)

[Mark Sheppard]

that's true wrt SO_REUSEPORT in MulticastSocket constructor. But the 
same could have been argued for the original
invocation of setReuseAddress, by default , in the constructors, which 
is encapsulating, what pereceived as, common or defacto

practice wrt applying SO_REUSEADDR on mcast sockets at the system level.
As I understand it, it is generally perceived that SO_REUSEADDR and 
SO_REUSEPORT are semantically equivalent for multicast sockets.
As such, I think in the case of MulticastSocket, the fact that the 
setRuseAddress() is called in the constructor, it is appropriate

to set SO_REUSEPORT also when it exists in the OS.

I take your point on the semantics of setReuseAddress in DatagramSocket 
as per its spec. The spec does allude to MulticastSocket.
As such, the current proposal's changes just lack the appropriate 
javadoc to describe its behavior, and its additional functionality of 
setting SO_REUSEPORT.
It is not necessary that overridden method should mirror the semantics 
of the base class method.
If it is accepted that it is generally perceived that SO_REUSEADDR and 
SO_REUSEPORT are semantically equivalent for multicast sockets,
then it seems appropriate that an overriding setReuseAddress(..) method 
in MulticastSocket can reflect this.


regards
Mark



On 14/09/2016 14:58, Chris Hegarty wrote:

One additional remark.

Was it appropriate to update the legacy MC constructors
to set the new JDK 9 SO_REUSEPORT in the first place?
This can be achievable, opt-in from new code, by creating
an unbound MS, setting the option, then binding.

-Chris.

On 14/09/16 14:47, Chris Hegarty wrote:

Mark,

On 14/09/16 14:22, Mark Sheppard wrote:


Hi Chris,
 I don't fully understand your objections to the approach taken.
Is there a compatibility issue with the addition of the additional
methods to MulticastSocket?


The concern is with setReuseAddress performing an operation that
is not clear from the method specification, e.g. from setReuseAddress()

 * Enable/disable the SO_REUSEADDR socket option.

This is no longer accurate. The proposed changes would affect
SO_REUSEPORT too.


I don't see Datagram.setReuseAddress(...) handling the SO_REUSEPORT
option, this has to be done explicitly via setOption at this level of
abstraction.


Yes, it is a bit odd, but these are legacy classes. I am not opposed
to adding a new method for this, or something else. I just want to
avoid future issues and confusion when setReuseAddress is called and
then it is noticed that, the somewhat orthogonal option, SO_REUSEPORT's
value has changed. setReuseAddress's spec is very clear about what it
does.


MulticastSocket is a subclass of DatagramSocket (that in itself is a
questionable structuring), and as such
has specialized behaviour, and part  of that specialization is the
setting of  the setting SO_REUSEADDR and SO_REUSEPORT
in its constructors, to enable address reuse semantics, prior to 
binding

an address.


Understood. Of course, the setting of SO_REUSEPORT is new in 9,
hence the problem.


As part of that specialization, it would seem appropriate that
MulticastSocket manipulate the SO_REUSEPORT
option in a consistent way. Adding an overridden setReuseAddress(...)
method provides that consistency and
encapsulates the specialized behaviour.


I agree with the abstraction, just not that setReuseAddress should
be used to achieve it. The name and spec of this method is so
tied to SO_REUSEADDR.


Is alternatively proposal to NOT do anything to MulticastSocket, BUT
document clearly how to handle the failing scenario, that an
MulticastSocket
requires both setReuseAddress() and a setOption call to disable
reuseaddress semantics on an unbound MulticastSocket ?


That is one option, and the option that I was suggesting as a possible
alternative.


This then raises the question of why have a convenience method, such as
setReuseAddress() in the first place, when it can be handled
adequately via the setOption


We are moving away from these option specific getter and setter
methods, in favor of the more general get/setOption methods, as
the latter are more adaptable.

If setReuseAddress is to operate on more than SO_REUSEADDR, then
its spec should be very clear about this.

-Chris.



regards
Mark

On 14/09/2016 13:34, Chris Hegarty wrote:

Mark,

On 14/09/16 13:23, Mark Sheppard wrote:


Hi Chris,
so are you accepting that it is correct to add the overridden
methods in MulticastSocket and that these need
appropriate javadoc ?


I think we need these, but they should just call their super
equivalents, i.e. no implicit setting of SO_REUSEPORT. They would
exist solely as a place to locate guidance, or a note, that one
will likely want to set SO_REUSEPORT too.

or are you advocating pushing the handing of the SO_REUSEPORT into 
the

base DatagramSocket class ?


It is already there. I am not proposing to change this.

It is not clear how your code changes fit in the proposed fix i.e. 
the

explicit setting of the option

Re: RFR 8153674: Expected SocketException not thrown when calling bind() with setReuseAddress(false)

[Mark Sheppard]

Hi Chris,
 I don't fully understand your objections to the approach taken.
Is there a compatibility issue with the addition of the additional 
methods to MulticastSocket?


I don't see Datagram.setReuseAddress(...) handling the SO_REUSEPORT 
option, this has to be done explicitly via setOption at this level of 
abstraction.


MulticastSocket is a subclass of DatagramSocket (that in itself is a 
questionable structuring), and as such
has specialized behaviour, and part  of that specialization is the 
setting of  the setting SO_REUSEADDR and SO_REUSEPORT
in its constructors, to enable address reuse semantics, prior to binding 
an address.
As part of that specialization, it would seem appropriate that 
MulticastSocket manipulate the SO_REUSEPORT
option in a consistent way. Adding an overridden setReuseAddress(...) 
method provides that consistency and

encapsulates the specialized behaviour.

Is alternatively proposal to NOT do anything to MulticastSocket, BUT 
document clearly how to handle the failing scenario, that an MulticastSocket
requires both setReuseAddress() and a setOption call to disable 
reuseaddress semantics on an unbound MulticastSocket ?



This then raises the question of why have a convenience method, such as 
setReuseAddress() in the first place, when it can be handled

adequately via the setOption

regards
Mark

On 14/09/2016 13:34, Chris Hegarty wrote:

Mark,

On 14/09/16 13:23, Mark Sheppard wrote:


Hi Chris,
so are you accepting that it is correct to add the overridden
methods in MulticastSocket and that these need
appropriate javadoc ?


I think we need these, but they should just call their super
equivalents, i.e. no implicit setting of SO_REUSEPORT. They would
exist solely as a place to locate guidance, or a note, that one
will likely want to set SO_REUSEPORT too.


or are you advocating pushing the handing of the SO_REUSEPORT into the
base DatagramSocket class ?


It is already there. I am not proposing to change this.


It is not clear how your code changes fit in the proposed fix i.e. the
explicit setting of the option to false?


My proposal is an alternative. It is not related to the current
webrev.


With the current proposed changes then I think it would be sufficient to
invoke setReuseAddress(true) in MulticastSocket constructors
rather than

// Enable SO_REUSEADDR before binding
setReuseAddress
<https://java.se.oracle.com/source/s?defs=setReuseAddress=jdk9-dev>(*true*); 



// Enable SO_REUSEPORT if supported before binding
*if* (supportedOptions
<https://java.se.oracle.com/source/xref/jdk9-dev/jdk/src/java.base/share/classes/java/net/MulticastSocket.java#supportedOptions>().contains 

<https://java.se.oracle.com/source/s?defs=contains=jdk9-dev>(StandardSocketOptions 

<https://java.se.oracle.com/source/s?defs=StandardSocketOptions=jdk9-dev>.SO_REUSEPORT 

<https://java.se.oracle.com/source/s?defs=SO_REUSEPORT=jdk9-dev>)) 
{

*this*.setOption
<https://java.se.oracle.com/source/s?defs=setOption=jdk9-dev>(StandardSocketOptions 

<https://java.se.oracle.com/source/s?defs=StandardSocketOptions=jdk9-dev>.SO_REUSEPORT 

<https://java.se.oracle.com/source/s?defs=SO_REUSEPORT=jdk9-dev>, 
*true*);

}


as the overridden setReuseAddress takes care of SO_REUSEPORT


Yes, this is what Vyom has proposed, in the webrev.

I would like to explore an alternative, so see what it would look like.

-Chris.



regards
Mark

On 14/09/2016 11:43, Chris Hegarty wrote:

Vyom,

On 11/09/16 08:01, Vyom Tewari wrote:

Hi All,

Please review the below code change.

Bug: https://bugs.openjdk.java.net/browse/JDK-8153674

Webrev  :
http://cr.openjdk.java.net/~vtewari/8153674/webrev0.0/index.html
<http://cr.openjdk.java.net/%7Evtewari/8153674/webrev0.0/index.html>

This change override the "get/setReuseAddress"  for MulticaseSocket 
and

will abstract with both reuse attributes(SO_REUSEADDR & SO_REUSEPORT).


This issue arises since the changes for 6432031 "Add support for
SO_REUSEPORT" [1], which sets SO_REUSEPORT on MulticastSocket, if
the available. So setting setReuseAddress(false) alone is no longer
sufficient to disable address/port reuse, one must also set
SO_REUSEPORT to false.

I would be really nervous about changing set/getReuseAddress, without
at least updating the javadoc to indicate that it is now, for MS,
operating on two socket options.  Although, I do have sympathy
here since SO_REUSEPORT and SO_REUSEADDR are almost identical when
dealing with multicasting.

An alternative, maybe; Since the MS constructors document that
SO_REUSEPORT will be set, where available, maybe a javadoc note
on the set/getReuseAddress methods would be sufficient, that
indicates that StandardSocketOptions#SO_REUSEPORT may also need
to be set to have the desired effect?

If so, then code would have to:

setReuseAddress(false);

if 
(supportedO

Re: RFR 8153674: Expected SocketException not thrown when calling bind() with setReuseAddress(false)

[Mark Sheppard]

Hi Chris,
so are you accepting that it is correct to add the overridden 
methods in MulticastSocket and that these need

appropriate javadoc ?

or are you advocating pushing the handing of the SO_REUSEPORT into the 
base DatagramSocket class ?


It is not clear how your code changes fit in the proposed fix i.e. the 
explicit setting of the option to false?


With the current proposed changes then I think it would be sufficient to 
invoke setReuseAddress(true) in MulticastSocket constructors

rather than

// Enable SO_REUSEADDR before binding
setReuseAddress 
(*true*);


// Enable SO_REUSEPORT if supported before binding
*if*  (supportedOptions 
().contains 
(StandardSocketOptions 
.SO_REUSEPORT 
)) {
*this*.setOption 
(StandardSocketOptions 
.SO_REUSEPORT 
,*true*);

}


as the overridden setReuseAddress takes care of SO_REUSEPORT

regards
Mark

On 14/09/2016 11:43, Chris Hegarty wrote:

Vyom,

On 11/09/16 08:01, Vyom Tewari wrote:

Hi All,

Please review the below code change.

Bug: https://bugs.openjdk.java.net/browse/JDK-8153674

Webrev  :
http://cr.openjdk.java.net/~vtewari/8153674/webrev0.0/index.html


This change override the "get/setReuseAddress"  for MulticaseSocket and
will abstract with both reuse attributes(SO_REUSEADDR & SO_REUSEPORT).


This issue arises since the changes for 6432031 "Add support for 
SO_REUSEPORT" [1], which sets SO_REUSEPORT on MulticastSocket, if

the available. So setting setReuseAddress(false) alone is no longer
sufficient to disable address/port reuse, one must also set
SO_REUSEPORT to false.

I would be really nervous about changing set/getReuseAddress, without
at least updating the javadoc to indicate that it is now, for MS,
operating on two socket options.  Although, I do have sympathy
here since SO_REUSEPORT and SO_REUSEADDR are almost identical when
dealing with multicasting.

An alternative, maybe; Since the MS constructors document that
SO_REUSEPORT will be set, where available, maybe a javadoc note
on the set/getReuseAddress methods would be sufficient, that
indicates that StandardSocketOptions#SO_REUSEPORT may also need
to be set to have the desired effect?

If so, then code would have to:

setReuseAddress(false);

if (supportedOptions().contains(StandardSocketOptions.SO_REUSEPORT))
this.setOption(StandardSocketOptions.SO_REUSEPORT, false);

  , but at least it is explicit.

Q: not all MS constructors document that SO_REUSEPORT is set, but
they should, right? This seems to have slipped past during 6432031 [1].

-Chris.

[1] https://bugs.openjdk.java.net/browse/JDK-6432031

Re: RFR 8075484:SocketInputStream.socketRead0 can hang even with soTimeout set

[Mark Sheppard]

my error ... i perceived NET_Timeout as a common function across all 
platforms
otherwise I would have suggested a NET_TimeoutWithCurrentTime for each 
platform.


providing a version of NET_Timeout with a recv function sound good, but
will require a version per platform (NET_TimeoutWithRecv) and such as a 
change will affect the network
code more extensively, otherwise you need to retain the NET_Timeout 
also,  as it is used in other part of native network.


This change is focused on SocketInputStream, but that is not only place 
where NET_Timeout is called.


either way there is the possibility of replicated code

regards
Mark






On 06/09/2016 13:55, Chris Hegarty wrote:

Vyom,


On 6 Sep 2016, at 10:20, Vyom Tewari <vyom.tew...@oracle.com> wrote:

Hi,

Please find the latest 
webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.2/index.html 
<http://cr.openjdk.java.net/%7Evtewari/8075484/webrev0.2/index.html>). I have 
incorporated the review comments.

Your changes completely avoid NET_Timeout, which is quite complex on
Linux, Mac, and AIX, for various reasons. ( NET_Timeout will use the
async close mechanism to signal/interrupt a thread blocked in a poll /
select ). Also, select is used on Mac, which will use poll after your
changes ( I do remember that there was a bug in poll on Mac around
the time of the original Mac port ).

Would is be better, rather than replicating the logic in  NET_Timeout,
to have a version that supports passing a function pointer, to run if
the poll/select returns before the timeout? Just an idea.

-Chris.


Thanks,

Vyom


On Monday 05 September 2016 08:37 PM, Chris Hegarty wrote:

On 05/09/16 15:37, Mark Sheppard wrote:

if the desire is to avoid making double calls on gettimeofday in the
NET_ReadWithTimeout's while loop for its main call flow,

Yes, the desire is to make no more calls to gettimeofday, than are
already done.


then consider a modification to NET_Timeout and create a version
int NET_TimeoutWithCurrentTime(int s, long timeout, stuct timeval *
current_time)

int NET_TimeoutWithCurrentTime(int s, long timeout, stuct timeval *
current_time) {
int result;
struct timeval t;
long prevtime, newtime;
struct pollfd pfd;
pfd.fd = s;
pfd.events = POLLIN;

if (timeout > 0) {
prevtime = (current_time->tv_sec * 1000)  +
current_time->tv_usec / 1000;
}

for(;;) {
result = poll(, 1, timeout);
if (result < 0 && errno == EINTR) {
if (timeout > 0) {
gettimeofday(, NULL);
newtime = (t.tv_sec * 1000)  +  t.tv_usec /1000;
timeout -= newtime - prevtime;
if (timeout <= 0)
return 0;
prevtime = newtime;
}
} else {
return result;
}
}
}

the NET_ReadWithTimeout is modified with.

   while (timeout > 0) {
result = NET_TimeoutWithCurrentTime(fd, timeout, );

in any case there is the potential for multiple invocation of
gettimeofday due to a need to make
poll restartable,

Yes, and that is fine. Just no more than is already there.

and adjust the originally specified timeout

accordingly, and for the edge case
after the non blocking read.

After an initial skim, your suggestion seems reasonable.

-Chris.


regards
Mark



On 05/09/2016 12:02, Chris Hegarty wrote:

Vyom,

webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.1/index.html

There is some concern about the potential performance effect, etc,
of gettimeofday, maybe there is a way out of this. The reuse of
NET_Timeout is good, but it also calls gettimeofday. It seems that
a specific NET_ReadWithTimeout could be written to NOT reuse
NET_Timeout, but effectively inline its interruptible operation.
Or write a variant of NET_Timeout that takes a function to
execute. Rather than effectively two loops conditioned on the
timeout.  Disclaimer: I have not actually tried to do this, but
it seems worth considering / evaluating.

-Chris.

On 02/09/16 04:39, Vyom Tewari wrote:

hi Dimitry,

thanks for review, I did consider to use  a monotonically increasing
clock like "clock_gettime" but  existing nearby code("NET_Timeout") uses
"gettimeofday"  so i choose to be consistent with the existing code.

Thanks,

Vyom


On Friday 02 September 2016 01:38 AM, Dmitry Samersoff wrote:

Vyom,

Did you consider to use select() to calculate timeout instead of
gettimeofday ?

gettimeofday is affected by system time changes, so running ntpd can
cause unpredictable behavior of this code. Also it's rather expensive
syscall.

-Dmitry

On 2016-09-01 19:03, Vyom Tewari wrote:

please find the updated
webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.1/index.html

<http://cr.openjdk.java.net/%7Evtewari/8075484/webrev0.1/index.html>).
I
incorporated the review comments.

Thanks,

Vyom


On Tuesday 30 August 2016 04:11 PM, Mark Sheppa

Re: RFR 8075484:SocketInputStream.socketRead0 can hang even with soTimeout set

[Mark Sheppard]

looks fine ... thanks

regards
Mark

On 06/09/2016 10:20, Vyom Tewari wrote:

Hi,

Please find the latest 
webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.2/index.html 
<http://cr.openjdk.java.net/%7Evtewari/8075484/webrev0.2/index.html>). 
I have incorporated the review comments.


Thanks,

Vyom


On Monday 05 September 2016 08:37 PM, Chris Hegarty wrote:

On 05/09/16 15:37, Mark Sheppard wrote:


if the desire is to avoid making double calls on gettimeofday in the
NET_ReadWithTimeout's while loop for its main call flow,


Yes, the desire is to make no more calls to gettimeofday, than are
already done.


then consider a modification to NET_Timeout and create a version
int NET_TimeoutWithCurrentTime(int s, long timeout, stuct timeval *
current_time)

int NET_TimeoutWithCurrentTime(int s, long timeout, stuct timeval *
current_time) {
int result;
struct timeval t;
long prevtime, newtime;
struct pollfd pfd;
pfd.fd = s;
pfd.events = POLLIN;

if (timeout > 0) {
prevtime = (current_time->tv_sec * 1000)  +
current_time->tv_usec / 1000;
}

for(;;) {
result = poll(, 1, timeout);
if (result < 0 && errno == EINTR) {
if (timeout > 0) {
gettimeofday(, NULL);
newtime = (t.tv_sec * 1000)  +  t.tv_usec /1000;
timeout -= newtime - prevtime;
if (timeout <= 0)
return 0;
prevtime = newtime;
}
} else {
return result;
}
}
}

the NET_ReadWithTimeout is modified with.

   while (timeout > 0) {
result = NET_TimeoutWithCurrentTime(fd, timeout, );

in any case there is the potential for multiple invocation of
gettimeofday due to a need to make
poll restartable,


Yes, and that is fine. Just no more than is already there.

 and adjust the originally specified timeout

accordingly, and for the edge case
after the non blocking read.


After an initial skim, your suggestion seems reasonable.

-Chris.


regards
Mark



On 05/09/2016 12:02, Chris Hegarty wrote:

Vyom,

>>>
webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.1/index.html 



There is some concern about the potential performance effect, etc,
of gettimeofday, maybe there is a way out of this. The reuse of
NET_Timeout is good, but it also calls gettimeofday. It seems that
a specific NET_ReadWithTimeout could be written to NOT reuse
NET_Timeout, but effectively inline its interruptible operation.
Or write a variant of NET_Timeout that takes a function to
execute. Rather than effectively two loops conditioned on the
timeout.  Disclaimer: I have not actually tried to do this, but
it seems worth considering / evaluating.

-Chris.

On 02/09/16 04:39, Vyom Tewari wrote:

hi Dimitry,

thanks for review, I did consider to use  a monotonically increasing
clock like "clock_gettime" but  existing nearby 
code("NET_Timeout") uses

"gettimeofday"  so i choose to be consistent with the existing code.

Thanks,

Vyom


On Friday 02 September 2016 01:38 AM, Dmitry Samersoff wrote:

Vyom,

Did you consider to use select() to calculate timeout instead of
gettimeofday ?

gettimeofday is affected by system time changes, so running ntpd can
cause unpredictable behavior of this code. Also it's rather 
expensive

syscall.

-Dmitry

On 2016-09-01 19:03, Vyom Tewari wrote:

please find the updated
webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.1/index.html 



<http://cr.openjdk.java.net/%7Evtewari/8075484/webrev0.1/index.html>). 


I
incorporated the review comments.

Thanks,

Vyom


On Tuesday 30 August 2016 04:11 PM, Mark Sheppard wrote:

Hi
   perhaps there is an opportunity to do  some refactoring here 
(...

for me a "goto " carries a code smell! )

along the lines

if (timeout) {
 nread =  NET_ReadWithTimeout(...);
} else {
  nread = NET_Read(...);
}


the NET_ReadWithTimeout (...) function will contain a
restructuring of
your goto loop
while (_timeout > 0) { nread = NET_Timeout(fd, _timeout);
   if (nread <= 0) {
   if (nread == 0) {
   JNU_ThrowByName(env, JNU_JAVANETPKG
"SocketTimeoutException",
   "Read timed out");
   } else if (nread == -1) {
   if (errno == EBADF) {
JNU_ThrowByName(env, JNU_JAVANETPKG
"SocketException", "Socket closed");
   } else if (errno == ENOMEM) {
JNU_ThrowOutOfMemoryError(env, "NET_Timeout
native heap allocation failed");
   } else {
JNU_ThrowByNameWithMessageAndLastError
   (env, JNU_JAVANETPKG "SocketException",
"select/poll failed");
   }
   }
 // release buffer in main call flow
//  if (bufP != BUF) {
//   

Re: RFR 8075484:SocketInputStream.socketRead0 can hang even with soTimeout set

[Mark Sheppard]

if the desire is to avoid making double calls on gettimeofday in the 
NET_ReadWithTimeout's while loop for its main call flow,

then consider a modification to NET_Timeout and create a version
int NET_TimeoutWithCurrentTime(int s, long timeout, stuct timeval * 
current_time)


int NET_TimeoutWithCurrentTime(int s, long timeout, stuct timeval * 
current_time) {

int result;
struct timeval t;
long prevtime, newtime;
struct pollfd pfd;
pfd.fd = s;
pfd.events = POLLIN;

if (timeout > 0) {
prevtime = (current_time->tv_sec * 1000)  + 
current_time->tv_usec / 1000;

}

for(;;) {
result = poll(, 1, timeout);
if (result < 0 && errno == EINTR) {
if (timeout > 0) {
gettimeofday(, NULL);
newtime = (t.tv_sec * 1000)  +  t.tv_usec /1000;
timeout -= newtime - prevtime;
if (timeout <= 0)
return 0;
prevtime = newtime;
}
} else {
return result;
}
}
}

the NET_ReadWithTimeout is modified with.

   while (timeout > 0) {
result = NET_TimeoutWithCurrentTime(fd, timeout, );

in any case there is the potential for multiple invocation of 
gettimeofday due to a need to make
poll restartable, and adjust the originally specified timeout 
accordingly, and for the edge case

after the non blocking read.

regards
Mark



On 05/09/2016 12:02, Chris Hegarty wrote:

Vyom,

>>> 
webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.1/index.html


There is some concern about the potential performance effect, etc,
of gettimeofday, maybe there is a way out of this. The reuse of
NET_Timeout is good, but it also calls gettimeofday. It seems that
a specific NET_ReadWithTimeout could be written to NOT reuse
NET_Timeout, but effectively inline its interruptible operation.
Or write a variant of NET_Timeout that takes a function to
execute. Rather than effectively two loops conditioned on the
timeout.  Disclaimer: I have not actually tried to do this, but
it seems worth considering / evaluating.

-Chris.

On 02/09/16 04:39, Vyom Tewari wrote:

hi Dimitry,

thanks for review, I did consider to use  a monotonically increasing
clock like "clock_gettime" but  existing nearby code("NET_Timeout") uses
"gettimeofday"  so i choose to be consistent with the existing code.

Thanks,

Vyom


On Friday 02 September 2016 01:38 AM, Dmitry Samersoff wrote:

Vyom,

Did you consider to use select() to calculate timeout instead of
gettimeofday ?

gettimeofday is affected by system time changes, so running ntpd can
cause unpredictable behavior of this code. Also it's rather expensive
syscall.

-Dmitry

On 2016-09-01 19:03, Vyom Tewari wrote:

please find the updated
webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.1/index.html 

<http://cr.openjdk.java.net/%7Evtewari/8075484/webrev0.1/index.html>). 
I

incorporated the review comments.

Thanks,

Vyom


On Tuesday 30 August 2016 04:11 PM, Mark Sheppard wrote:

Hi
   perhaps there is an opportunity to do  some refactoring here (...
for me a "goto " carries a code smell! )

along the lines

if (timeout) {
 nread =  NET_ReadWithTimeout(...);
} else {
  nread = NET_Read(...);
}


the NET_ReadWithTimeout (...) function will contain a 
restructuring of

your goto loop
while (_timeout > 0) { nread = NET_Timeout(fd, _timeout);
   if (nread <= 0) {
   if (nread == 0) {
   JNU_ThrowByName(env, JNU_JAVANETPKG
"SocketTimeoutException",
   "Read timed out");
   } else if (nread == -1) {
   if (errno == EBADF) {
JNU_ThrowByName(env, JNU_JAVANETPKG
"SocketException", "Socket closed");
   } else if (errno == ENOMEM) {
   JNU_ThrowOutOfMemoryError(env, "NET_Timeout
native heap allocation failed");
   } else {
JNU_ThrowByNameWithMessageAndLastError
   (env, JNU_JAVANETPKG "SocketException",
"select/poll failed");
   }
   }
 // release buffer in main call flow
//  if (bufP != BUF) {
//  free(bufP);
// }
  nread = -1;
  break;
   } else {
nread = NET_NonBlockingRead(fd, bufP, len);
if (nread == -1 && ((errno == EAGAIN) || (errno == EWOULDBLOCK))) {
   gettimeofday(, NULL);
newtime = t.tv_sec * 1000 + t.tv_usec / 1000;
_timeout -= newtime - prevtime;
if(_timeout > 0){
prevtime = newtime;
   }
} else { break; } } } return nread;

e

regards
Mark


On 29/08/2016 10:58, Vyom Tewari wrote:

gentle reminder, please review the below code change.

Vyom


On Monday 22 August 2016 05:12 PM, Vyom Tewa

Re: Ping: RFR(S): 8163181: Further improvements for Unix NetworkInterface native implementation

[Mark Sheppard]

builds and regression tests appear to be OK with the proposed changes.

regards
Mark

On 02/09/2016 12:41, Mark Sheppard wrote:


have had a look through the changes twice, and they look fine ... i'll 
apply the patch and run a regression build to confirm


the moving of int flags on 919 to a conditional block,  I expect to 
cause a strict compile error in my solaris env, so need to check that


regards
Mark

On 02/09/2016 08:22, Langer, Christoph wrote:


Hi (Mark or Chris?),

as this RFR is outstanding for quite a while and it merely is a minor 
change which only cleans up coding, would you mind to review it that 
I can get it off my list?


The only major thing probably is, that calls to getMacAddress don’t 
need a socket anymore and sockets are only opened for platforms where 
an ioctl is done to determine the MacAddress.


As I’ve said, I’ve tested it on the Linux/Unix platforms.

Thanks a lot

Christoph

*From:*Langer, Christoph
*Sent:* Montag, 22. August 2016 15:30
*To:* 'net-dev@openjdk.java.net' <net-dev@openjdk.java.net>
*Subject:* RE: Ping: RFR(S): 8163181: Further improvements for Unix 
NetworkInterface native implementation


Hi,

I made a little update to my change: 
http://cr.openjdk.java.net/~clanger/webrevs/8163181.2/ 
<http://cr.openjdk.java.net/%7Eclanger/webrevs/8163181.2/>


Merely comments and a minor AIX specific correction.

Thanks in advance for reviewing

Christoph

*From:*Langer, Christoph
*Sent:* Dienstag, 16. August 2016 14:49
*To:* 'net-dev@openjdk.java.net' <net-dev@openjdk.java.net 
<mailto:net-dev@openjdk.java.net>>
*Subject:* Ping: RFR(S): 8163181: Further improvements for Unix 
NetworkInterface native implementation


Ping: Can I get a review for this small set of changes please?

Thanks

Christoph

*From:*Langer, Christoph
*Sent:* Donnerstag, 4. August 2016 15:04
*To:* net-dev@openjdk.java.net <mailto:net-dev@openjdk.java.net>
*Subject:* RFR(S): 8163181: Further improvements for Unix 
NetworkInterface native implementation


Hi,

I had made a few more cleanups when I was working on 
NetworkInterface.c which I thought are worth contributing.


Please review: http://cr.openjdk.java.net/~clanger/webrevs/8163181.1/ 
<http://cr.openjdk.java.net/%7Eclanger/webrevs/8163181.1/>


Bug: https://bugs.openjdk.java.net/browse/JDK-8163181 
<https://bugs.openjdk.java.net/browse/JDK-8163181>


As always, changes were built and tested on Linux, AIX, Solaris and Mac.

Thanks

Christoph

Re: Ping: RFR(S): 8163181: Further improvements for Unix NetworkInterface native implementation

[Mark Sheppard]

have had a look through the changes twice, and they look fine ... i'll 
apply the patch and run a regression build to confirm


the moving of int flags on 919 to a conditional block,  I expect to 
cause a strict compile error in my solaris env, so need to check that


regards
Mark

On 02/09/2016 08:22, Langer, Christoph wrote:


Hi (Mark or Chris?),

as this RFR is outstanding for quite a while and it merely is a minor 
change which only cleans up coding, would you mind to review it that I 
can get it off my list?


The only major thing probably is, that calls to getMacAddress don’t 
need a socket anymore and sockets are only opened for platforms where 
an ioctl is done to determine the MacAddress.


As I’ve said, I’ve tested it on the Linux/Unix platforms.

Thanks a lot

Christoph

*From:*Langer, Christoph
*Sent:* Montag, 22. August 2016 15:30
*To:* 'net-dev@openjdk.java.net' 
*Subject:* RE: Ping: RFR(S): 8163181: Further improvements for Unix 
NetworkInterface native implementation


Hi,

I made a little update to my change: 
http://cr.openjdk.java.net/~clanger/webrevs/8163181.2/ 



Merely comments and a minor AIX specific correction.

Thanks in advance for reviewing

Christoph

*From:*Langer, Christoph
*Sent:* Dienstag, 16. August 2016 14:49
*To:* 'net-dev@openjdk.java.net' >
*Subject:* Ping: RFR(S): 8163181: Further improvements for Unix 
NetworkInterface native implementation


Ping: Can I get a review for this small set of changes please?

Thanks

Christoph

*From:*Langer, Christoph
*Sent:* Donnerstag, 4. August 2016 15:04
*To:* net-dev@openjdk.java.net 
*Subject:* RFR(S): 8163181: Further improvements for Unix 
NetworkInterface native implementation


Hi,

I had made a few more cleanups when I was working on 
NetworkInterface.c which I thought are worth contributing.


Please review: http://cr.openjdk.java.net/~clanger/webrevs/8163181.1/ 



Bug: https://bugs.openjdk.java.net/browse/JDK-8163181 



As always, changes were built and tested on Linux, AIX, Solaris and Mac.

Thanks

Christoph

Re: RFR 8075484:SocketInputStream.socketRead0 can hang even with soTimeout set

[Mark Sheppard]

Hi
  perhaps there is an opportunity to do  some refactoring here (... for 
me a "goto " carries a code smell! )


along the lines

if (timeout) {
nread =  NET_ReadWithTimeout(...);
} else {
 nread = NET_Read(...);
}


the NET_ReadWithTimeout (...) function will contain a restructuring of 
your goto loop


while (_timeout > 0) { nread = NET_Timeout(fd, _timeout);
  if (nread <= 0) {
  if (nread == 0) {
  JNU_ThrowByName(env, JNU_JAVANETPKG "SocketTimeoutException",
  "Read timed out");
  } else if (nread == -1) {
  if (errno == EBADF) {
   JNU_ThrowByName(env, JNU_JAVANETPKG "SocketException", 
"Socket closed");
  } else if (errno == ENOMEM) {
  JNU_ThrowOutOfMemoryError(env, "NET_Timeout native heap 
allocation failed");
  } else {
  JNU_ThrowByNameWithMessageAndLastError
  (env, JNU_JAVANETPKG "SocketException", "select/poll 
failed");
  }
  }
// release buffer in main call flow
//  if (bufP != BUF) {
//  free(bufP);
// }
 nread = -1;
 break;
  } else {
nread = NET_NonBlockingRead(fd, bufP, len);
if (nread == -1 && ((errno == EAGAIN) || (errno == EWOULDBLOCK))) {
  gettimeofday(, NULL);
newtime = t.tv_sec * 1000 + t.tv_usec / 1000;
_timeout -= newtime - prevtime;
if(_timeout > 0){
prevtime = newtime;
  }
} else { break; } } } return nread;


e

regards
Mark


On 29/08/2016 10:58, Vyom Tewari wrote:

gentle reminder, please review the below code change.

Vyom


On Monday 22 August 2016 05:12 PM, Vyom Tewari wrote:

Hi All,

Please review the code changes for below issue.

Bug : https://bugs.openjdk.java.net/browse/JDK-8075484

webrev: 
http://cr.openjdk.java.net/~vtewari/8075484/webrev0.0/index.html 



This issue is SocketInputStream.socketread0() hangs even with 
"soTimeout" set.the implementation of 
Java_java_net_SocketInputStream_socketRead0 assumes that read() won't 
block after poll() reports that a read is possible.


This assumption does not hold, as noted on the man page for select 
(referenced by the man page for poll): Under Linux, select() may 
report a socket file descriptor as "ready for reading", while 
nevertheless a subsequent read blocks. This could for example happen 
when data has arrived but upon examination has wrong checksum and is 
discarded.


Thanks,

Vyom

Re: RFR 8158023: SocketExceptions contain too little information sometimes

[Mark Sheppard]

Hi Christoph,
thanks for the response ... yes, you do check the 
getLastErrorString return ... sorry about that!


regards
Mark

On 27/06/2016 14:28, Langer, Christoph wrote:

Hi Mark,

thanks for looking at this. Please see my comments inline.


  wrt JNU_ThrowByNameWithMessaheAndLastError, it would appear that it
doesn't allow for malloc to fail and hence
str1 could be null  and a problematic input to jio_snprintf which could
result in a de-referencing of zero.

You are right, thanks. I should check for null and return "OutOfMemory" 
appropriately. I fixed that and updated the webrev in place.


in the call flow would it not be more appropriate to manipulate native
buffers first to produce the desired error string and
then allocate the java string object?
c.f. src/java.base/unic/native/libnet/net_util_md.c
NET_ThrowUnknownHostExceptionWithGaiError

Well, in the case of NET_ThrowUnknownHostExceptionWithGaiError you will 
probably have the hostname as well as the error text encoded in platform 
encoding. So for that it is perfectly fine to do the native buffers first and 
then convert to a Java String. But in my case for 
JNU_ThrowByNameWithMessageAndLastError, the errno text will be in platform 
encoding but the other message and formatting will be UTF-8 strings from the 
executable. So I have to handle both parts differently when creating a String 
object out of it. Or am I wrong in my assumption here?


should you allow for getLastErrorString not to return an error string or
return an error result?
as in JNU_ThrowByNameWithLastError

I do, don't I? I'm checking the return value of getLastErrorString: "if (n > 
0)".

Best regards
Christoph


On 27/06/2016 08:42, Langer, Christoph wrote:

Hi,

eventually here is the - hopefully final - version of this fix:
http://cr.openjdk.java.net/~clanger/webrevs/8158023.3/

Now I leave JNU_ThrowByNameWithLastError untouched and I've also added

conversion to the new function JNU_ThrowByNameWithMessageAndLastError.
I've replaced JNU_ThrowByNameWithLastError with
JNU_ThrowByNameWithMessageAndLastError in the java/net coding where I
think it is appropriate (mostly in occasions when a SocketException is thrown
kind of generically). @Paul: thanks for your suggestion regarding the output
format but I would still prefer an output like "java.net.SocketException: ioctl
SIOCGSIZIFCONF failed: Bad file number" vs. " java.net.SocketException: Bad
file number (ioctl SIOCGSIZIFCONF failed)" as I'm always handing down a
message to the new throw routine.

Hoping for a review :)

Best regards
Christoph


-Original Message-
From: Kenji Kazumura [mailto:k...@jp.fujitsu.com]
Sent: Mittwoch, 8. Juni 2016 02:51
To: Langer, Christoph 
Cc: net-dev@openjdk.java.net; nio-...@openjdk.java.net; core-libs-
d...@openjdk.java.net
Subject: Re: RFR 8158023: SocketExceptions contain too little information
sometimes

Christoph,

You should not remove conversion codes (platform string to Java String)
at JNU_ThrowByNameWithLastError,
and you have to add conversion codes at
JNU_ThrowByNameWithMessageAndLastError.

It seems that you assume strerror returns only ascii characters, but actually
not.
It depends on the locale of your environment where java programs runs.


-Kenji Kazumura


In message

 RFR 8158023: SocketExceptions contain too little information sometimes
 "Langer, Christoph"  wrote:


Hi all,

please review the following change:
Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8158023.1/
Bug: https://bugs.openjdk.java.net/browse/JDK-8158023

During error analysis I stumbled over a place where I encountered a

SocketException which was thrown along with some strerror information as
message. I found it hard to find the originating code spot with that

information.

So I looked at the places where we throw exceptions, namely JNU_Throw...

and NET_Throw... functions and came up with the following enhancement:

- NET_ThrowByNameWithLastError can go completely as it does not

provide

any benefit over JNU_ThrowByNameWithLastError.

- JNU_ThrowByNameWithLastError can be cleaned up.

- I added JNU_ThrowByNameWithMessageAndLastError to print out a

string

like message + ": " + last error.

- I went over all places where NET_ThrowByNameWithLastError is used

and

replaced it appropriately.

Do you think this change is desirable/possible?

Though it's mainly a net topic, I'm posting it to nio-dev and core-libs-dev as

well as JNU_Throw... code affects all.

Best regards
Christoph

Re: RFR 8158023: SocketExceptions contain too little information sometimes

[Mark Sheppard]

Hi,
wrt JNU_ThrowByNameWithMessaheAndLastError, it would appear that it 
doesn't allow for malloc to fail and hence
str1 could be null  and a problematic input to jio_snprintf which could 
result in a de-referencing of zero.


in the call flow would it not be more appropriate to manipulate native 
buffers first to produce the desired error string and

then allocate the java string object?
c.f. src/java.base/unic/native/libnet/net_util_md.c
NET_ThrowUnknownHostExceptionWithGaiError

should you allow for getLastErrorString not to return an error string or 
return an error result?

as in JNU_ThrowByNameWithLastError

regards
Mark

On 27/06/2016 08:42, Langer, Christoph wrote:

Hi,

eventually here is the - hopefully final - version of this fix:
http://cr.openjdk.java.net/~clanger/webrevs/8158023.3/

Now I leave JNU_ThrowByNameWithLastError untouched and I've also added conversion to the new 
function JNU_ThrowByNameWithMessageAndLastError. I've replaced JNU_ThrowByNameWithLastError with 
JNU_ThrowByNameWithMessageAndLastError in the java/net coding where I think it is appropriate 
(mostly in occasions when a SocketException is thrown kind of generically). @Paul: thanks for your 
suggestion regarding the output format but I would still prefer an output like 
"java.net.SocketException: ioctl SIOCGSIZIFCONF failed: Bad file number" vs. " 
java.net.SocketException: Bad file number (ioctl SIOCGSIZIFCONF failed)" as I'm always handing 
down a message to the new throw routine.

Hoping for a review :)

Best regards
Christoph


-Original Message-
From: Kenji Kazumura [mailto:k...@jp.fujitsu.com]
Sent: Mittwoch, 8. Juni 2016 02:51
To: Langer, Christoph 
Cc: net-dev@openjdk.java.net; nio-...@openjdk.java.net; core-libs-
d...@openjdk.java.net
Subject: Re: RFR 8158023: SocketExceptions contain too little information
sometimes

Christoph,

You should not remove conversion codes (platform string to Java String)
at JNU_ThrowByNameWithLastError,
and you have to add conversion codes at
JNU_ThrowByNameWithMessageAndLastError.

It seems that you assume strerror returns only ascii characters, but actually
not.
It depends on the locale of your environment where java programs runs.


-Kenji Kazumura


In message

RFR 8158023: SocketExceptions contain too little information sometimes
"Langer, Christoph"  wrote:


Hi all,

please review the following change:
Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8158023.1/
Bug: https://bugs.openjdk.java.net/browse/JDK-8158023

During error analysis I stumbled over a place where I encountered a

SocketException which was thrown along with some strerror information as
message. I found it hard to find the originating code spot with that 
information.

So I looked at the places where we throw exceptions, namely JNU_Throw...

and NET_Throw... functions and came up with the following enhancement:

- NET_ThrowByNameWithLastError can go completely as it does not provide

any benefit over JNU_ThrowByNameWithLastError.

- JNU_ThrowByNameWithLastError can be cleaned up.

- I added JNU_ThrowByNameWithMessageAndLastError to print out a string

like message + ": " + last error.

- I went over all places where NET_ThrowByNameWithLastError is used and

replaced it appropriately.

Do you think this change is desirable/possible?

Though it's mainly a net topic, I'm posting it to nio-dev and core-libs-dev as

well as JNU_Throw... code affects all.

Best regards
Christoph

Re: RFR [9] 8085785: sun/net/www/protocol/http/ZoneId.java timeout intermittently

[Mark Sheppard]

Hi Chris,
   this looks good  ... so the server now listens on wildcard and the 
client uses IPv6 loopback as the destination address.
The use of NO_PROXY, is good. I wouldn't have thought of that, and in 
the past, Tests have experienced firewall issues on

linux and macos perviously without this setting.

regards
Mark

On 26/05/2016 14:33, Chris Hegarty wrote:

ZoneId is attempting to verify the 'Host' header set by the HttpURLConnection 
implementation
when given an IPv6 literal containing a scope id. It does so by iterating the 
network interfaces
on the machine attempting to find one that is suitable to use as the listening 
interface for a
temporary test HTTP sever. Then it connects to it, and verifies the value of 
the 'Host' header.

This is problematic as some interfaces like teredo on Windows, or awdl on Mac, 
are not
suitable. Rather than all this, the test can use the IPv6 loopback address, 
which can contain
a scope id, if retrieved from the NetworkInterface API.

http://cr.openjdk.java.net/~chegar/8085785/
https://bugs.openjdk.java.net/browse/JDK-8085785

-Chris.