Code Review Request: 8028562

[Xuelei Fan]

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
>