RFR 15 8243099: SO_INCOMING_NAPI_ID support

[Alan Bateman]

On 04/05/2020 22:21, Ivanov, Vladimir A wrote:
> Thanks for your comments.
>
> Updated version of the patch available as http://cr.openjdk.java.net/~sviswanathan/Vladimir/8243099/webrev.07/
> The exception part for the setOption method was updated
> +                } else if (option == SO_INCOMING_NAPI_ID) {
> +                    if (!incomingNapiIdOptSupported)
> +                        throw new UnsupportedOperationException("Attempt to set unsupported option " + option);
> +                    else
> +                        throw new IOException("Attempt to set read only option " + option);
>
> It required to update the method's description. As side effect some application may report compile error for the 'setOption' method usage while one more
> exception should be added to processing.
>
> The Javadoc and the ExtOptionNAPITest.java were updated to use IOE instead of UOE.
>
> Tests "test/jdk/jdk/net/Sockets test/jdk/java/net/SocketOption/AfterClose.java test/jdk/java/nio/channels/etc/PrintSupportedOptions.java" passed on the
> RHEL8 (with NAPI support) and the RHEL7.7 (without NAPI support).
A SocketException is an IOException so no need for both in the setOption 
declaration. The method is internal so it's not going to impact anyone 
using any of the supported APIs. That said, it might be more consistent 
to throw SocketException here and that will avoid needing to change the 
declarations.

So assuming s/IOException/SocketException/ then I think the javadoc 
looks okay. A residual question from the discussion is whether you want 
to make it clear that the identifier is a positive Integer. Once we have 
the javadoc agreed here then you should get the CSR submitted.

I don't think ExtOptionNAPITest is ready for review. I think we need a 
simpler test that is reliable and easy to maintain. It will need to 
exercise each of the network channels (defined in java.nio.channels) in 
addition to the socket APIs defined in java.net. It is important that 
the listener binds to an ephemeral port as we can't used fixed ports in 
tests. It's also important that all tests do cleanup (the current 
ExtOptionNAPITest leaves many sockets open). We can iterate on the test 
here in parallel with the CSR.

-Alan