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

[Alan Bateman]

On 17/04/2018 06:26, Hamlin Li wrote:
>
> :
>
> Yes, you're right too.
> But, the root cause for "pollConnected changed the channel state to 
> ST_CONNECTED" is that pollConnect only check whether return result of 
> Net.poll(...) > 0, it does not check whether it is/includes POLLERR, 
> POLLHUP, POLLNVAL, so endFinishConnect is passed true for parameter 
> completed when events is/includes POLLERR, POLLHUP, POLLNVAL, this 
> will cause "state = ST_CONNECTED;" in endFinishConnect.
> To fix this, completed should be calculated separately from polled, so 
> when POLLERR, POLLHUP, POLLNVAL is included in result of Net.poll(), 
> polled == true, but completed == false, endFinishConnect will not set 
> "state = ST_CONNECTED;". In this way, there is no need to treat 
> pollConnect differently from other pollXXX, and pass false explicitly 
> to completed parameter of endFinishConnect.
> Hope I have explained clearly.
It's important that the pollXXX methods return true whenever any events, 
including POLLERR or POLLHUP, are polled. Consider the following:

Socket s = sc.socket();
s.setSoTimeout(10*1000);
int n = s.getInputStream().read(b);

Assuming no bytes are available, this will block in poll. Now suppose 
the connection is terminated, say RST. In that case, you should see a 
POLLHUP. It's important that the pollRead returns true so the socket 
adaptor can attempt a read and throw the exception. If POLLHUP is 
ignored then this will spin.

I know you aren't suggesting it returning false but it's important to 
understand that before coming to the Thread.interrupt case (which is 
what I think you are really asking about). If the thread is interrupted 
when blocked in poll then the begin/end around the blocking call will 
ensure that the channel is closed. This will cause the poll to wakeup. 
It doesn't matter if pollXXX throws AsynchronousCloseException or not as 
the socket adaptor will follow it with a read or finishConnect and that 
will cause the IOException to be thrown. In other words, the value 
passed to end doesn't really matter. It did matter in the 
endFinishConnect case because that caused the channel state to change 
and we don't want that.

As I said in the other mail, it might be better if we add 
beginPoll/endPoll methods that the pollXXX methods could use instead of 
the beginRead/beginFinishConnect etc. That would make it a bit easier to 
understand.

-Alan