Hello!
I reverted the direction of the patch, and now TwoStacks implementation
is changed to mimic DualStack.
As part of this fix, I also added another @run line to several tests to
execute them with the option -Djava.net.preferIPv4Stack=true to make
sure both implementations work as expected.
After this patch the diff shows difference in only hundred lines of the
two implementations, so it will be easy to merge them together on the
next step.
Here's the set of webrev:
WEBREV-000: http://cr.openjdk.java.net/~igerasim/8198358/02/000/webrev/
remove overridden TwoStacksPlainSocketImpl.close().
The only difference from the base AbstractPlainSocketImpl was that the
base first calls socketPreClose() and then socketClose(), and there's no
difference is these two on Windows.
WEBREV-001: http://cr.openjdk.java.net/~igerasim/8198358/02/001/webrev/
rename initProto to initIDs
WEBREV-002: http://cr.openjdk.java.net/~igerasim/8198358/02/002/webrev/
changing socketBind
WEBREV-003: http://cr.openjdk.java.net/~igerasim/8198358/02/003/webrev/
changing socketCreate and socketListen
WEBREV-004: http://cr.openjdk.java.net/~igerasim/8198358/02/004/webrev/
changing socketAvailable
WEBREV-005: http://cr.openjdk.java.net/~igerasim/8198358/02/005/webrev/
changing socketClose0
WEBREV-006: http://cr.openjdk.java.net/~igerasim/8198358/02/006/webrev/
changing socketShutdown
WEBREV-007: http://cr.openjdk.java.net/~igerasim/8198358/02/007/webrev/
changing socketSendUrgentData
WEBREV-008: http://cr.openjdk.java.net/~igerasim/8198358/02/008/webrev/
changing socketAccept
WEBREV-009: http://cr.openjdk.java.net/~igerasim/8198358/02/009/webrev/
changing socketConnect
WEBREV-010: http://cr.openjdk.java.net/~igerasim/8198358/02/010/webrev/
delegating to super.getOption(), overriding socketGetOption
WEBREV-011: http://cr.openjdk.java.net/~igerasim/8198358/02/011/webrev/
changing socketSetOption
WEBREV-012: http://cr.openjdk.java.net/~igerasim/8198358/02/012/webrev/
changing socketGetOption
WEBREV-013: http://cr.openjdk.java.net/~igerasim/8198358/02/013/webrev/
removing unnecessary global variables
WEBREV-014: http://cr.openjdk.java.net/~igerasim/8198358/02/014/webrev/
network tests to run with -Djava.net.preferIPv4Stack=true
For reference, here's the combined webrev:
http://cr.openjdk.java.net/~igerasim/8198358/03/webrev/
With kind regards,
Ivan
On 3/9/18 3:50 AM, Chris Hegarty wrote:
Ivan,
Thanks for breaking up the changes, it makes it easier to review.
I disagree with changing DualStackPlainSocketImp to conform with
TwoStacks (and unix/PlainSocketImpl). I would prefer to move the
latter to the former. Specifically, around the use of fdAccess.
-Chris.
On 06/03/18 20:31, Ivan Gerasimov wrote:
In order to make is easier to review the fix, I made the webrev.ksh
to generate a series of incremental webrevs from the mq patch stack.
Here's the list of the incremental changes with a brief comments:
WEBREV-000: http://cr.openjdk.java.net/~igerasim/8198358/00/000/webrev/
Only changing the order of methods declaration
WEBREV-001: http://cr.openjdk.java.net/~igerasim/8198358/00/001/webrev/
Renaming initIDs to initProto
WEBREV-002: http://cr.openjdk.java.net/~igerasim/8198358/00/002/webrev/
Changing socketBind
WEBREV-003: http://cr.openjdk.java.net/~igerasim/8198358/00/003/webrev/
Changing socketCreate
WEBREV-004: http://cr.openjdk.java.net/~igerasim/8198358/00/004/webrev/
Changing socketListen
WEBREV-005: http://cr.openjdk.java.net/~igerasim/8198358/00/005/webrev/
Changin socketAvailable
WEBREV-006: http://cr.openjdk.java.net/~igerasim/8198358/00/006/webrev/
Changing socketClose0
WEBREV-007: http://cr.openjdk.java.net/~igerasim/8198358/00/007/webrev/
Changing socketShutdown
WEBREV-008: http://cr.openjdk.java.net/~igerasim/8198358/00/008/webrev/
Changing socketSendUrgentData
WEBREV-009: http://cr.openjdk.java.net/~igerasim/8198358/00/009/webrev/
Changing socketAccept
WEBREV-010: http://cr.openjdk.java.net/~igerasim/8198358/00/010/webrev/
Changing socketConnect
WEBREV-011: http://cr.openjdk.java.net/~igerasim/8198358/00/011/webrev/
Minor editing, comments, moving code
WEBREV-012: http://cr.openjdk.java.net/~igerasim/8198358/00/012/webrev/
Changing socketSetOption
WEBREV-013: http://cr.openjdk.java.net/~igerasim/8198358/00/013/webrev/
Changing socketGetOption
WEBREV-014: http://cr.openjdk.java.net/~igerasim/8198358/00/014/webrev/
Moving a few methods one more time
Accumulative webrev with all the changes above is available here:
http://cr.openjdk.java.net/~igerasim/8198358/01/webrev/
Thanks in advance!
Ivan
On 3/1/18 8:43 PM, Ivan Gerasimov wrote:
Hello!
I'd like to do the next step toward removing the TwoStacks socket
implementation on Windows.
It would be aligning the two implementations (DualStack and
TwoStacks), so they can be easier merged together during the next step.
There are three PlainSocketImpl implementations in JDK:
java.base/windows/classes/java/net/DualStackPlainSocketImpl.java
java.base/windows/classes/java/net/TwoStacksPlainSocketImpl.java
java.base/unix/classes/java/net/PlainSocketImpl.java
While two later have very similar organization (in particular, set
of native methods), the former is organized slightly differently.
In order to merge the two Windows implementation together, they
first need to be organized in a similar way.
For consistency, DualStack implementation will be reorganized to be
aligned with TwoStacks and unix/PlainSocketImpl.
Bug: https://bugs.openjdk.java.net/browse/JDK-8198358
Webrev: http://cr.openjdk.java.net/~igerasim/8198358/00/webrev/
The change looks somewhat messy, but in fact it was a series of
incremental changes, which I still keep in the mercurial 'mq'.
(I wish the webrev could be made incremental based on the mq
patches, to make it easier to review.)
The patched JDK builds fine and all the regression tests pass Okay.
Thanks in advance!
--
With kind regards,
Ivan Gerasimov
