Code Review Request: 8028562

[zaiyao liu]

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