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

[Hamlin Li]

On 16/04/2018 7:26 PM, Alan Bateman wrote:
> On 16/04/2018 12:02, Hamlin Li wrote:
>> 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.
> No, the root cause is was that pollConnected changed the channel state 
> to ST_CONNECTED. 
Hi Alan,

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.

Thank you
-Hamlin
> We don't want the pollXXX methods changing the channel state. It only 
> came about because they are re-using the beginXXX/endXXX methods to 
> wrap the blocking poll. It might be clearer if we just inlined those.
>
> -Alan