Re: 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