8201474: (so) Socket adaptor connect(InetAddress, timeout) succeeds when connection fails

[Hamlin Li]

Hi Alan,

Sorry for the delayed comments, I did not have chance to check the changes.

I think the fix is good, it should work, but I think we can do better.

The root cause of 8201474 is that in pollXXX(e.g. pollConnected in 
SocketChannelImpl.java) only check whether return result of 
Net.poll(...) > 0, it does not check whether it is/includes POLLERR, 
POLLHUP, POLLNVAL. But in fact, pollXXX should check further even if 
Net.poll(...) > 0, in case of POLLERR, POLLHUP, POLLNVAL, completed 
should be set as false.

So I file a bug https://bugs.openjdk.java.net/browse/JDK-8201588 to 
improve what's done in 8201474, please check the patch at 
http://cr.openjdk.java.net/~mli/8201588/webrev.00, which is based on the 
jdk revision before 8201474, I can send out review based on 8201474 
later if necessary.

How do you think about it?


Thank you

-Hamlin



On 12/04/2018 8:48 PM, Alan Bateman wrote:
> This is a regression in JDK 11 where a java.net.Socket is obtained 
> from a SocketChannel and its 2-arg connect method is used to attempt a 
> connection and the connect fails. The implementation incorrectly sets 
> the state to connected for this case. The 1-arg (no timeout) connect 
> doesn't have this issue.
>
> For the timeout case, the socket adaptor uses pollConnected. This is 
> blocking so the poll is wrapped by beginFinishConnect/end to support 
> interrupt and async/close. The bug is that endFinishConnect is called 
> with completed=true when any events are polled, including error 
> events, so the connection state is set to indicate that the connection 
> is established. The changes are simple:
>
> http://cr.openjdk.java.net/~alanb/8201474/webrev/index.html
>
> (Thank you to whoever reported JDK-8201474).
>
> -Alan