Code Review Request: 8028562

[Xuelei Fan]

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
>