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

[Hamlin Li]

On 17/04/2018 2:50 PM, Alan Bateman wrote:
> 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.
Hi Alan,

I think we both understand each other, but choose different ways, that's 
fine, as you know my concerns.
Thank you for discussing.

-Hamlin
>
> -Alan
>
>
>