[rfc][icedtea-web] PR835: javaws findBestUrl leaks TCP connections

Adam Domurad adomurad at redhat.com
Thu Feb 14 12:09:03 PST 2013


On 02/13/2013 05:37 PM, Omair Majid wrote:
> Hi Adam,
>
> On 02/11/2013 01:03 PM, Adam Domurad wrote:
>> Turns out it wasn't so great :-/
>> Turns out properly reusing HttpURLConnection is quite confusing :-/.
>>
>> OK apparently disconnect calls should be avoided. See David Lloyd's
>> comment @ http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=835.
> Thanks for the pointer.
>
>> +            if (finalLocation == null) {
>> +                System.err.println("Attempted to download " + resource.location + ", but failed to connect!");
>> +            }
>> +
> I am not sure if this is trying to handle finalLocation == null case or
> just an informative message. If this is trying to handle finalLocation
> being null, it doesn't look complete. A NullPointerException will
> probably be thrown a few lines after:

It's just information that gets consumed otherwise. Added an explicit throw.

>
>>               URLConnection connection = finalLocation.openConnection(); // this won't change so should be okay unsynchronized
>> +    private static int getResourceUrlResponseCode(URL url, String requestMethod) throws IOException {
>> +        URLConnection connection = url.openConnection();
>> +        connection.addRequestProperty("Accept-Encoding", "pack200-gzip, gzip");
>> +
> One nit: this method has no information about encodings, but proceeds to
> add the Accept-Encoding header. This looks out of place and mixes
> concerns. Maybe you can modify this method to take the request
> properties? Or maybe this method can take a just-opened URLConnection?
>
>> +            httpConnection.getInputStream().close();
> It's not clear to me from the docs that this reads/consumes the input. I
> assume you have verified this.
>
>> +                if (responseCode == 501 /* 501 response signifies Not-Implemented */) {
> Maybe compare responseCode with
> java.net.HttpURLConnection.HTTP_NOT_IMPLEMENTED ?
>
> Cheers,
> Omair
>




More information about the distro-pkg-dev mailing list