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
