Code Review Request: 8028562

[Sean Mullan]

Sorry for the late comment, but there is a typo in this comment (roud -> 
round):

// will try to read one more roud when read error

I suggest rewording this to:

// will try to read one more time if there is a read error

Also, it is too late to push this for JDK 8 as it is not critical, so 
you will have to wait until the JDK 9 repo opens ...

Thanks,
Sean

On 12/04/2013 07:45 PM, zaiyao liu wrote:
> Hi Xuelei,
>
> Can you help to submit this change to repository? I don't have openjdk
> account.
>
> Thanks
>
> Kevin
> On 2013/12/4 15:41, Xuelei Fan wrote:
>> Looks fine to me.
>>
>> Xuelei
>>
>> On 12/4/2013 3:24 PM, zaiyao liu wrote:
>>> Hi Xuelei,
>>>
>>> I have updated, please
>>> review:http://cr.openjdk.java.net/~ewang/kevin/JDK-8028562/webrev.00/
>>> <http://cr.openjdk.java.net/%7Eewang/kevin/JDK-8028562/webrev.00/>
>>>
>>> Thanks
>>>
>>> Kevin
>>> On 2013/12/4 14:50, Xuelei Fan wrote:
>>>> On 12/4/2013 2:36 PM, zaiyao liu wrote:
>>>>> Hi Xuelei,
>>>>>
>>>>> Thanks for you suggestion. please review again:
>>>>> http://cr.openjdk.java.net/~ewang/kevin/JDK-8028562/webrev.00/
>>>>>
>>>> Need a white space:
>>>> - 224   //will try to read one more roud when read error
>>>> + 224   // will try to read one more roud when read error
>>>>
>>>> The message is not clear enough:
>>>> - 302  log("will read one more round");
>>>> + 302  log("Need to read more from client");
>>>>
>>>> Otherwise, looks fine to me.  Please go ahead.
>>>>
>>>> Thanks,
>>>> Xuelei
>>>>
>>>>> Kevin
>>>>> On 2013/12/4 12:06, Xuelei Fan wrote:
>>>>>> On 12/4/2013 11:33 AM, zaiyao liu wrote:
>>>>>>> Hi Xuelei,
>>>>>>>
>>>>>>> Can you help to review again.
>>>>>>> http://cr.openjdk.java.net/~ewang/kevin/JDK-8028562/webrev.00/
>>>>>>>
>>>>>> Thanks for the update.  Please pay attentions to the code
>>>>>> conversions.
>>>>>>
>>>>>>     300 if (serverIn.remaining() != clientMsg.length) {
>>>>>>     301     if(retry){
>>>>>>     302        log("will read one more round");
>>>>>>
>>>>>> It might be reasonable to retry when "serverIn.remaining()" less than
>>>>>> clientMsg.length", what do you think?
>>>>>>
>>>>>>
>>>>>> Xuelei
>>>>>>
>>>>>>
>>>>>>> Thanks
>>>>>>>
>>>>>>> Kevin
>>>>>>> On 2013/12/3 19:50, Xuelei Fan wrote:
>>>>>>>> On 12/3/2013 6:59 PM, zaiyao liu wrote:
>>>>>>>>> Hi Xuelei,
>>>>>>>>>
>>>>>>>>> I can't reproduce this issue after run 900 times at windows and
>>>>>>>>> linux
>>>>>>>>> platform,
>>>>>>>> It should be pretty hard to reproduce the issue in normal TCP/IP
>>>>>>>> environment.
>>>>>>>>
>>>>>>>>> for this fix just run one more round after get exception.
>>>>>>>>>
>>>>>>>>> please review:
>>>>>>>>> http://sqeweb.us.oracle.com/net/sqenfs-1/export1/comp/jsn/users/kevin1/webrev/8028562/webrev/
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>> I don't think it is the expected fix.  Looks like the underlying
>>>>>>>> issue
>>>>>>>> is that "serverOut.remaining() == 0" (line 282) does not always
>>>>>>>> mean
>>>>>>>> the
>>>>>>>> server has received all of the client message (line 298,
>>>>>>>> (serverIn.remaining() != clientMsg.length)).  I would suggest
>>>>>>>> run one
>>>>>>>> more round (at line 241) after server message delivered
>>>>>>>> ("serverOut.remaining() == 0" (line 282)).
>>>>>>>>
>>>>>>>> The logic looks like, in runTest(boolean):
>>>>>>>> loop (line 241):
>>>>>>>>        read client message
>>>>>>>>        send server message
>>>>>>>>        if server delivered all server message {
>>>>>>>>            if server received all client message {
>>>>>>>>                check the message
>>>>>>>>            } else {
>>>>>>>>                loop one more time, go to "loop" (only one time?).
>>>>>>>>            }
>>>>>>>>        }
>>>>>>>>
>>>>>>>> Hope it helps.
>>>>>>>>
>>>>>>>> Xuelei
>>>>>>>>
>>>>>>>>
>>>>>>>>> Thanks
>>>>>>>>>
>>>>>>>>> Kevin
>