RFR 8224477 [13] java.net.Socket::setOption - implementation mismatch to spec

[Alan Bateman]

On 23/05/2019 17:32, Chris Hegarty wrote:
> Thank you Alan,
>
> I believe that I addressed all your comments and suggestions.
>
> Additionally, while here I noticed an issue where these methods were not
> always consistent with their spec to throw IOException when the socket
> has been closed.
>
> https://cr.openjdk.java.net/~chegar/8224477/webrev.02/
>
The isClosed checks look fine although a Socket/SS/DS could be 
asynchronously closed so needs to be handled by the impl when the Socket 
doesn't provide the coordination with close. This is a long standing 
rats nest of course, and should go away with the new implementation.

The new AfterClose and the now renamed NullsAndBadValues seems very 
comprehensive.

I have a few comments on the new setOption/getOption implementation:

- The value == null check isn't needed as it will be caught by the 
isInstance(value) check

- I think the checks in StdSocketOptionUtil.throwIfBadValue should be 
moved into SocketImpl.setOption and DatagramSocketImpl.setOption.  That 
is, these methods map new to old and I think are the right place to 
reject values that should throw IAE. This should avoid the override need 
to validate the values before delegating to the super class.

- I think SocketImpl.setOption needs the same 
name.type().isInstance(value) check. Not important but would ensure that 
any pre JDK 9 SocketImpls throw the right exception (the issue of 
SocketImpl.setOption not specifying IAE is another issue of course).

-Alan