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

Omair Majid omajid at redhat.com
Wed Feb 13 14:37:08 PST 2013


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:

>              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

-- 
PGP Key: 66484681 (http://pgp.mit.edu/)
Fingerprint = F072 555B 0A17 3957 4E95  0056 F286 F14F 6648 4681



More information about the distro-pkg-dev mailing list