RFR [11] 8198358 : Align organization of DualStackPlainSocketImpl with TwoStacksPlainSocketImp [win]

[Ivan Gerasimov]

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