You are using the master repository. I think Dev is expected to used the dev repository, http://hg.openjdk.java.net/jdk9/dev.
No additional code review required, I think. I will integrated the changeset. Thanks, Xuelei On 12/16/2013 10:45 AM, zaiyao liu wrote: > 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 >> >
