Request for review: 8009650 - HttpClient available() check throws SocketException when connection has been closed

Chris Hegarty chris.hegarty at oracle.com
Wed Mar 13 11:38:09 PDT 2013


Thank you Rob.

-Chris

On 13 Mar 2013, at 18:02, Rob McKenna <rob.mckenna at oracle.com> wrote:

> Thanks Kurchi, Chris, Dmitry,
> 
> I'm planning to fix that testcase and to make the logger final before integration.
> 
>    -Rob
> 
> On 13/03/13 17:55, Kurchi Hazra wrote:
>> I looked at the source code changes, and it looks good.
>> 
>> Thanks,
>> - Kurchi
>> 
>> 
>> On 3/13/2013 7:42 AM, Chris Hegarty wrote:
>>> The source code changes look fine to me.
>>> 
>>> I'm not sure why you enabled a security manager in the test. I don't think that it needs one. You can remove the explicit setting of the SM from the test code, remove the policy file, and the also the jtreg policy tag. Otherwise looks fine.
>>> 
>>> -Chris.
>>> 
>>> On 13/03/2013 12:53, Dmitry Samersoff wrote:
>>>> Rob,
>>>> 
>>>> Looks good for me.
>>>> 
>>>> -Dmitry
>>>> 
>>>> On 2013-03-13 04:35, Rob McKenna wrote:
>>>>> Hi folks,
>>>>> 
>>>>> New webrev at:
>>>>> 
>>>>> http://cr.openjdk.java.net/~robm/8009650/webrev.02/
>>>>> 
>>>>> Apologies for the delay.
>>>>> 
>>>>>     -Rob
>>>>> 
>>>>> On 07/03/13 23:19, Rob McKenna wrote:
>>>>>> Ah, I see what you mean. Can do.
>>>>>> 
>>>>>>     -Rob
>>>>>> 
>>>>>> On 07/03/13 23:13, Dmitry Samersoff wrote:
>>>>>>> Rob,
>>>>>>> 
>>>>>>> Sorry for not being clean enough. We have repeated pattern:
>>>>>>> 
>>>>>>>   if (logger.isLoggable(PlatformLogger.FINEST)) {
>>>>>>>       logger.finest("HttpClient.available(): " + msg
>>>>>>>   }
>>>>>>> 
>>>>>>> so it makes code better readable if we can put it to some common place.
>>>>>>> 
>>>>>>> -Dmitry
>>>>>>> 
>>>>>>> On 2013-03-08 02:31, Rob McKenna wrote:
>>>>>>>> Hi Dmitry,
>>>>>>>> 
>>>>>>>> I'm not 100% sure what you mean by duplication, the exceptions and
>>>>>>>> their
>>>>>>>> messages are distinct. I think it would be best to keep it that way.
>>>>>>>> 
>>>>>>>>      -Rob
>>>>>>>> 
>>>>>>>> On 07/03/13 22:00, Dmitry Samersoff wrote:
>>>>>>>>> Rob,
>>>>>>>>> 
>>>>>>>>> Is it possible to avoid code duplication?
>>>>>>>>> 
>>>>>>>>> i.e. do something like this:
>>>>>>>>> 
>>>>>>>>>      int r;
>>>>>>>>> 
>>>>>>>>>      try {
>>>>>>>>>     ...
>>>>>>>>>      } catch (SocketException e) {
>>>>>>>>>       // Comments goes here
>>>>>>>>>       r = -1
>>>>>>>>>      }
>>>>>>>>> 
>>>>>>>>>      if (r == -1){
>>>>>>>>>         if (logger. ...
>>>>>>>>>         available = false;
>>>>>>>>>      }
>>>>>>>>> 
>>>>>>>>>     return available;
>>>>>>>>> 
>>>>>>>>> -Dmitry
>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> On 2013-03-07 20:18, Rob McKenna wrote:
>>>>>>>>>> Hi folks,
>>>>>>>>>> 
>>>>>>>>>> This is a slight alteration of the fix contributed by Stuart Douglas.
>>>>>>>>>> This fix deals with a SocketException caused by getSoTimeout() on a
>>>>>>>>>> closed connection.
>>>>>>>>>> 
>>>>>>>>>> http://cr.openjdk.java.net/~robm/8009650/webrev.01/
>>>>>>>>>> 
>>>>>>>>>>       -Rob
> 



More information about the net-dev mailing list