Code Review Request: 8028562

[zaiyao liu]

Hi Sean,

Thanks for your suggestion, updated please review: 
http://cr.openjdk.java.net/~ewang/kevin/JDK-8028562/webrev.00/

Thanks again,

Kevin
On 2013/12/6 1:13, Sean Mullan wrote:
> 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
>>
>