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

[Adam Domurad]

On 02/07/2013 06:04 PM, Omair Majid wrote:
> On 02/07/2013 05:58 PM, Adam Domurad wrote:
>> On 02/07/2013 05:10 PM, Omair Majid wrote:
>>> On 02/07/2013 04:45 PM, Adam Domurad wrote:
>>>> Tiny patch here.
>>>>
>>>> This issue still exists in head:
>>>> http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=835
>>>>
>>>> The patch in the bug report works, although I have preferred the use of
>>>> a finally block (I prefer the stronger guarantee).
>>>>
>>>> ChangeLog:
>>>>
>>>> 2013-XX-XX  Adam Domurad  <adomurad at redhat.com>
>>>>
>>>>       * netx/net/sourceforge/jnlp/cache/ResourceTracker.java
>>>>       (findBestUrl): Ensure that connections are not leaked.
>>> Looks okay to me.
>>>
>>> As an improvement, maybe we can avoid closing and return the "best"
>>> connection so it can be reused without having to establish another one.
>>>
>>> Cheers,
>>> Omair
>>>
>> Good idea. Luckily the code was set-up to make this fairly easy.
>>
>> Due to rearranging the disconnect is placed slightly different (more
>> aligned with the original patch).
>>
>> Note that if connection == null, the NPE will get caught. I didn't have
>> an idea of a better exception to throw, but I figured this should be
>> noted (it had caused me pains before).
> Looks great.
>
> Cheers,
> Omair
>

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.

This new patch instead takes the route of first trying a HEAD request, 
which will prevent the entire file being sent (causing a connection to 
stick around). If it receives 501 == not-implemented, it attempts a GET 
request with equivalent behaviour to before.

ChangeLog
2013-XX-XX  Adam Domurad  <adomurad at redhat.com>
     PR835: javaws leaks connections
     Uses HEAD requests if possible for testing URLs. Adds name to download
     threads.
     * netx/net/sourceforge/jnlp/cache/ResourceTracker.java:
     (startThread): Add name to download thread
     (getResourceUrlResponseCode): Get or fake an HTTP response code.
     (findBestUrl): Use getResourceUrlResponseCode to first try a HEAD
     request. Fall-back to GET rquest.

I have bundled a small change to give a name to the downloader threads.

Happy hacking,
-Adam

-------------- next part --------------
A non-text attachment was scrubbed...
Name: PR835.patch
Type: text/x-patch
Size: 4951 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20130211/f8381fb1/PR835.patch