Hi Andrew and Sean,
Please help to review,this webrev based on JDK9:
http://cr.openjdk.java.net/~pzhang/Kevin/8028562/webrev/
Thanks and Regards.
Kevin
On 2013/12/16 9:40, zaiyao liu wrote:
Just record this email in security-dev alias.
On 2013/12/6 18:23, Xuelei Fan wrote:
On 12/6/2013 6:04 PM, Xuelei Fan wrote:
On 12/6/2013 5:11 PM, zaiyao liu wrote:
Hi Xuelei,
Can you help me to post the webrev into cr.openjdk.java.net if you
think fine, I don't have account, I asked Eric to do this before,
but he
has left office.
Sure, but I'm not sure whether JDK 9 repository is open. I will check
it and push the fix if the repository opened.
I think the JDK 9 repository has not open. We have to wait for a while.
Xuelei
Thanks,
Xuelei
Thanks so much.
Kevin
On 2013/12/6 16:51, Xuelei Fan wrote:
Hi Kevin,
Please won't mind there is a lot comments back-and-forth for such a
simple fix. That's the way we (dev) are working for better quality of
such a widely deployed platform.
As this is the first bug you addressed in dev lib, I think it
might be
help to have a good start rather than just fix the bug quickly.
After
you working over one or two JDK release, we may just say "looks
fine to
me" for simple fixes. ;-)
Hope you understand.
Xuelei
On 12/6/2013 4:41 PM, Xuelei Fan wrote:
I just noticed this. "read error" does not describe the issue
properly.
The underlying issue is that client message may be fragmented
into
small pieces during the TCP transaction. It's reasonable although
it is
pretty hard to reproduce.
How about:
- // will try to read one more time if there is a read error
+ // will try to read one more time in case client message
+ // is fragmented to multiple pieces
Xuelei
On 12/6/2013 3:45 PM, zaiyao liu wrote:
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
