Code review request 7183373: URLClassloader.close() does not close JAR files whose resources have been loaded via getResource()

[Shirish Kuncolienkar]

On 8/24/2012 9:32 PM, Shirish Kuncolienkar wrote:
> On 8/24/2012 5:39 PM, Michael McMahon wrote:
>> On 23/08/12 18:50, Shirish Kuncolienkar wrote:
>>> Could I get the change reviewed please
>>>
>>> This behavior is seen on Windows.
>>> Logic in URLClassPath.getLoader() does not take care of an URL which 
>>> looks like "jar:file:/C:/test/xyz.jar!/". The logic ends up choosing 
>>> a FileLoader instead of a JarLoader.  JarLoader has provision for 
>>> closing file handles, so choosing a JarLoader will solve the 
>>> problem. Secondly the constructor of JarLoader blindly adds a prefix 
>>> and suffix to the provided URL to make it look like a jar URL. 
>>> Changed the code here to conditionally append/prepend
>>>
>>> The change set can be found at 
>>> http://cr.openjdk.java.net/~shirishk/7183373/webrev.0/
>>>
>>> -Shirish
>>>
>> Shirish,
>>
>> I have a slight concern that this would modify the Loader class to be 
>> used in some circumstances
>> completely independent of the requirements of URLClassLoader.close(). 
>> This is very sensitive code.
>> Would it be possible to fix this within the context of whatever 
>> loader is currently being invoked?
>>
>> - Michael
>>
> Michael,
>
> Thanks for the review comments.  The second version of the fix is 
> uploaded at http://cr.openjdk.java.net/~shirishk/7183373/webrev.2/
> Could you please take a look at this one ?
>
> Description of the fix:
> URLClassPath.Loader.findResource() method opens a connection to the 
> provided URL to test whether the URL is good.  Here the Jar file gets 
> opened but does not get closed because the created stream as 
> setUseCaches set to true.
>
> Just out of curiosity I would like to know bit more on "some 
> circumstances completely independent of the requirements of 
> URLClassLoader.close()".  I see that the Loader classes are private in 
> nature and are being used within the context of the URLClassPath.
> We create an instance of JarLoader for all the jars that are on the 
> extension class loader path by adding "jar" ,  "!/" to the file url 
> which comes as the input.  The reason behind the first fix was that if 
> we have a url like this why not use a JarLoader instance.
>
> - Shirish
>
Michael,

Did you get a chance to look at the new patch?

-Shirish