RFR 8200181 [11] Remove superflous non-IPv4 code from Java_java_net_TwoStacksPlainSocketImpl_socketListen

[Ivan Gerasimov]

Hi Chris!


On 3/23/18 11:48 AM, Chris Hegarty wrote:
> Ivan,
>
> On 23/03/18 18:38, Ivan Gerasimov wrote:
>> Hi Chris!
>>
>> I was about to submit a patch for 8198358 that heavily modifies 
>> TwoStacksPlainSocketImpl to make it aligned with 
>> DualStackPlainSocketImpl.
>>
>> That patch will also remove this problematic code.
>>
>> I can rebase my patch, but wouldn't it be easier to proceed with that 
>> fix?
>
> There is an immediate urgency to have this possibly problematic
> code removed. We're seeing strangle rare intermittent failures
> that may be caused by it, and possibly the socket cleaner. It
> is extremely difficult to reproduce, so we want to eliminate
> this code from the equation.
>
Okay, got it.
I can rebase my patch after you push your fix.

> Regarding 8198358, I've been thinking again about this, and I
> think a better approach may be to just remove
> TwoStackPlainSocketImpl completely. The preferIPv4Stack can go
> through DualStackPlainSocketImpl with a indicator to create an
> AF_INET socket, just like on unix. I think this could be a more
> straight forward path forward, unless testing shows otherwise.
>
Right.  That's what I wanted to approach, but in a step-by-step way.
Here's the list of other differences that remained after aligning the 
implementations:
- in waitForConnect() I had to add throwing  ConnectException if the 
last error was WSAEADDRNOTAVAIL (there was a regression test failure, 
when I tried to remove this special case),
- in accept(), had to check the new file descriptor for == -2 (in the 
DualStacks the last error code is checked, which may be wrong),
- had to add handling SO_TIMEOUT (in DualStack it's just ignored, but 
there was a test failure if I tried the same with TwoStacks).

I agree that it looks too much work, given we're going to remove 
TwoStacks anyways, but I found it easier to do it this way to avoid the 
regressions.

With kind regards,
Ivan

> Maybe 8198358 could be amended to do this? Is this something that
> you would be interested in doing? Otherwise, I can try some
> experiments to see how it looks.
>
> -Chris.
>
>> I'm going send the webrev set shortly.
>>
>> With kind regards,
>>
>> Ivan
>>
>>
>> On 3/23/18 10:18 AM, Chris Hegarty wrote:
>>> Given that JDK-8058965 removed the IPv6 support from
>>> TwoStacksPlainSocketImpl, the native socketListen method no longer
>>> needs to deal with the non-IPv4 code path. This simplifies the code,
>>> brings it in line with the unix variants, and removes a possible
>>> problematic code path that could close the socket without notifying
>>> the socket cleaner.
>>>
>>> http://cr.openjdk.java.net/~chegar/8200181.00/
>>> https://bugs.openjdk.java.net/browse/JDK-8200181
>>>
>>> -Chris.
>>>
>>> [1] https://bugs.openjdk.java.net/browse/JDK-8058965
>>>
>>
>

-- 
With kind regards,
Ivan Gerasimov