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

2013-01-24 Thread Chris Hegarty
On 24 Jan 2013, at 02:54, Frank Ding wrote: > Hi Kurchi, > > Thanks. Is it eligible for committing? Yes, you have sufficient reviews so can commit. -Chris > > Best regards, > Frank > > On 1/23/2013 4:45 PM, Kurchi Subhra Hazra wrote: >> Looks good to me. >> >> Thanks, >> Kurchi >> >>

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

2013-01-23 Thread Frank Ding
Hi Kurchi, Thanks. Is it eligible for committing? Best regards, Frank On 1/23/2013 4:45 PM, Kurchi Subhra Hazra wrote: Looks good to me. Thanks, Kurchi On 1/22/13 6:50 PM, Frank Ding wrote: Hi Chris and Kurchi, Thank you both for your comments. I have reformatted the patch accordingly

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

2013-01-23 Thread Chris Hegarty
On 23/01/2013 08:45, Kurchi Subhra Hazra wrote: Looks good to me. +1 Thanks Frank, -Chris. Thanks, Kurchi On 1/22/13 6:50 PM, Frank Ding wrote: Hi Chris and Kurchi, Thank you both for your comments. I have reformatted the patch accordingly @ http://cr.openjdk.java.net/~dingxmin/7183373/w

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

2013-01-23 Thread Kurchi Subhra Hazra
Looks good to me. Thanks, Kurchi On 1/22/13 6:50 PM, Frank Ding wrote: Hi Chris and Kurchi, Thank you both for your comments. I have reformatted the patch accordingly @ http://cr.openjdk.java.net/~dingxmin/7183373/webrev.03/ Please take a look again :-) Best regards, Frank On 1/22/2

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

2013-01-22 Thread Frank Ding
Hi Chris and Kurchi, Thank you both for your comments. I have reformatted the patch accordingly @ http://cr.openjdk.java.net/~dingxmin/7183373/webrev.03/ Please take a look again :-) Best regards, Frank On 1/22/2013 4:42 AM, Chris Hegarty wrote: On 01/21/2013 08:36 PM, Kurchi Subhra Ha

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

2013-01-21 Thread Chris Hegarty
On 01/21/2013 08:36 PM, Kurchi Subhra Hazra wrote: The change looks consistent with what we had already (findResource() will now silently consume an UnknownServiceException instead of a NullPointerException for MailToURLConnection). Also, I noticed that the test does not fail on Mac OS X even wi

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

2013-01-21 Thread Kurchi Subhra Hazra
The change looks consistent with what we had already (findResource() will now silently consume an UnknownServiceException instead of a NullPointerException for MailToURLConnection). Also, I noticed that the test does not fail on Mac OS X even without the fix - it does fail on Windows though. T

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

2013-01-21 Thread Frank Ding
Hi Chris, Thanks for your review. I did jtreg test on net module (java/net and sun/net) and no regression issue was found. Could anybody else review the patch as well? Best regards, Frank On 1/18/2013 10:55 PM, Chris Hegarty wrote: I haven't been able to spend as much time on this as I wo

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

2013-01-18 Thread Chris Hegarty
I haven't been able to spend as much time on this as I would like, jdk8 M6 code freeze is approaching fast. Since this area is fraught with danger the safest change would be what is in the .2 version of the webrev [1]. I think I would be ok with this. -Chris. [1] http://cr.openjdk.java.net/~s

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

2013-01-17 Thread Frank Ding
Hi Michael, Could you please take a look at my comment below? Best regards, Frank On 1/6/2013 4:23 PM, Frank Ding wrote: Hi Michael, After reading carefully discussion thread, let me elaborate my investigation and conclusion. The 2nd version of Shirish's patch tries to address your conce

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

2013-01-06 Thread Frank Ding
Hi Michael, After reading carefully discussion thread, let me elaborate my investigation and conclusion. The 2nd version of Shirish's patch tries to address your concern that "Would it be possible to fix this within the context of whatever loader is currently being invoked?". The new solut

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

2012-08-28 Thread 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 lik

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

2012-08-24 Thread Shirish Kuncolienkar
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

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

2012-08-24 Thread Michael McMahon
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.

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

2012-08-23 Thread Shirish Kuncolienkar
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 hand