On 17/12/2019 14:35, Patrick Concannon wrote:
Hi,
Could someone please review my fix and CSR for issue JDK-8235783
'DatagramSocket::disconnect should allow an implementation to throw
UncheckedIOException'?
DatagramChannel's connect and disconnect methods throw an IOException
while DatagramSocket's corresponding methods do not. The proposed fix
changes the specification of DatagramSocket::connect and
DatagramSocket::disconnect to document that an implementation may
throw UncheckedIOException.
In addition, and similar to what was done for DatagramChannel, an
@apiNote is added to DatagramSocket::disconnect to recommend closing
the socket if an IO exception occurs, as the underlying socket might
have been left in an inconsistent and unspecified state
bug: https://bugs.openjdk.java.net/browse/JDK-8235783
CSR: https://bugs.openjdk.java.net/browse/JDK-8236076
webrev: http://cr.openjdk.java.net/~pconcannon/8235783/webrevs/webrev.00/
A couple of things in this area to think about:
1. DatagramSocket connect does nothing when the socket is closed and we
might want specify that some day to avoid the reader wondering if
UncheckedIOException will be thrown when the socket is closed.
2. DatagramSocket connect will bind the socket is not already bound and
we might want to specify that some day. The bind could fail so that is a
potential reason for throwing UncheckedIOException (or SecurityException
is unlikely scenarios).
3. DatagramSocket.connect silently falls back to emulation when the
underlying connect fails. I think this is a bug as it hides a problem
like trying to connect to a non-routable addresses, but may have been
done this way to avoid incomplete DatagramSocketImpl implementations. I
guess my point here is that UncheckedIOException will never be thrown,
which might be okay if the primary motive is to prepare DatagramSocket
for the new implementation.
4. If you are open to word smithing on the @throws descriptions then I
think dropping "by an implementation" will help the wording a bit. Also
I think the description should make it clear that the exception cause is
the IOException with the underlying I/O error.
-Alan.
