Code Review Request: 8028562

zaiyao liu zaiyao.liu at oracle.com
Wed Dec 4 07:24:43 UTC 2013


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




More information about the security-dev mailing list