[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