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 <daniel.fu...@oracle.com>
Sent: Tuesday 8 October 2019 09:16
To: mark sheppard <macanao...@hotmail.com>; Alan Bateman
<alan.bate...@oracle.com>; nio-dev <nio-...@openjdk.java.net>
Cc: OpenJDK Network Dev list <net-dev@openjdk.java.net>
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 <net-dev-boun...@openjdk.java.net> on behalf of Daniel
> Fuchs <daniel.fu...@oracle.com>
> *Sent:* Monday 7 October 2019 11:34
> *To:* Alan Bateman <alan.bate...@oracle.com>; nio-dev
> <nio-...@openjdk.java.net>
> *Cc:* OpenJDK Network Dev list <net-dev@openjdk.java.net>
> *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.
>
