Thank you, Daniel
Regards,
Chris
> On 19 Nov 2018, at 5:40 PM, Daniel Fuchs <daniel.fu...@oracle.com> wrote:
>
> Looks good to me Chris!
>
> best regards,
>
> -- daniel
>
> On 19/11/2018 03:03, Chris Yin wrote:
>> Hi, Daniel
>>> On 16 Nov 2018, at 7:13 PM, Daniel Fuchs <daniel.fu...@oracle.com> wrote:
>>>
>>> Hi Chris,
>>>
>>> This looks good to me, though I wonder if the check
>>> for reason.length() > 0 should be made in
>>>
>>> 51 public synchronized String failureReason() {
>>> 52 return reason.length() > 0 ? reason.toString() : null;
>>> 53 }
>>>
>>> instead of within passed() just to be on the safe side, in case
>>> some subclass is tricked into thinking that there is a failure
>>> if failureReason() starts returning "" instead of null?
>> Thanks for your suggestion, yes, that’s make sense since the original logic
>> “return reason” may also return null if no failure message, update webrev as
>> below.
>> http://cr.openjdk.java.net/~xyin/8129310/webrev.03/
>> Thanks,
>> Chris
>>>
>>> best regards,
>>>
>>> -- daniel
>>>
>>> On 16/11/2018 03:58, Chris Yin wrote:
>>>> Sorry, one more revision here since I found we could do a little more in
>>>> subclasses (use loopback address to avoid possible outside noise), and
>>>> this will introduce the test class which extends AsyncCloseTest into
>>>> webrev, hope that will be convenient for review, thanks
>>>> http://cr.openjdk.java.net/~xyin/8129310/webrev.02/
>>>> Regards,
>>>> Chris
>>>>> On 16 Nov 2018, at 10:39 AM, Chris Yin <xu.y....@oracle.com> wrote:
>>>>>
>>>>> Hi, Daniel
>>>>>
>>>>> Thanks a lot for your reviewing and comments, revision webrev as below.
>>>>>
>>>>> http://cr.openjdk.java.net/~xyin/8129310/webrev.01/
>>>>>
>>>>> Regards,
>>>>> Chris
>>>>>
>>>>>> On 15 Nov 2018, at 6:06 PM, Daniel Fuchs <daniel.fu...@oracle.com> wrote:
>>>>>>
>>>>>> Hi chris,
>>>>>>
>>>>>> There's no reason to make `passed` and `closed` volatile since
>>>>>> they are private and only accessed from withing synchronized
>>>>>> methods.
>>>>>>
>>>>>> On the other hand, reason could now be final.
>>>>>>
>>>>>> Otherwise I guess it looks good. I haven't looked at the tests that
>>>>>> use AsyncClose so I trust that you have verified that your changes
>>>>>> work as you expect.
>>>>>>
>>>>>> best regards,
>>>>>>
>>>>>> -- daniel
>>>>>>
>>>>>> On 15/11/2018 09:12, Chris Yin wrote:
>>>>>>> Please review below minor change for
>>>>>>> java/net/Socket/asyncClose/AsyncClose.java fails intermittently issue,
>>>>>>> thanks
>>>>>>> From previous failure log, seems lack of enough info to find out root
>>>>>>> cause, this change is to enhance a little to base test class
>>>>>>> AsyncCloseTest with guessing, even it may not completely address the
>>>>>>> issue, additional log info should still helpful for future debugging.
>>>>>>> 1. add volatile to 'boolean passed' and 'boolean closed' to avoid any
>>>>>>> visibility issue since they were write/read in different threads
>>>>>>> 2. failure reason info may be overwritten if failed() been called
>>>>>>> multiple times, change it to record all failure messages.
>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8129310
>>>>>>> webrev: http://cr.openjdk.java.net/~xyin/8129310/webrev.00/
>>>>>>> Regards,
>>>>>>> Chris
>>>>>>
>>>>>
>>>
>
