[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