Re: RFR: 6947916: JarURLConnection does not handle useCaches correctly

[Chris Hegarty]

> On 9 Sep 2016, at 19:35, Rob McKenna  wrote:
> 
> To be explicit, new webrev here:
> 
> http://cr.openjdk.java.net/~robm/6947916/webrev.03/

Looks fine.

-Chris.

P.S. I’m ok if you want to explicitly delete the file either, just use the test 
library
FileUtils convenient method.


>   -Rob
> 
> On 09/09/16 07:03, Rob McKenna wrote:
>> Chris just pointed out to me that the test.classes prefix on the jar path is 
>> unnecessary. He also mentioned that jtreg would clear up the scratch 
>> directory so the deleteOnExit wouldn't be needed either.
>> 
>>  -Rob
>> 
>> On 09/09/16 05:02, Rob McKenna wrote:
>>> Will do
>>> 
>>> -Rob
>>> 
>>> On 09/09/16 11:00, Roger Riggs wrote:
 Hi Rob,
 
 Looks ok.
 
 Its also a good practice to cleanup the file.  (File.deleteOnExit).
 
 Roger
 
 
 On 9/9/2016 9:23 AM, Rob McKenna wrote:
> Hey Chris,
> 
> Apologies, I'm guilty of "just doing what adjacent tests do" here.
> 
> That shell script is actually there in the test source already, but 
> generating the jar from the test means theres no need to use it or to 
> check in a binary. Thanks for picking me up!
> 
> http://cr.openjdk.java.net/~robm/6947916/webrev.02/
> 
>   -Rob
> 
> On 08/09/16 08:40, Chris Hegarty wrote:
>>> On 7 Sep 2016, at 14:17, Rob McKenna  wrote:
>>> 
>>> Hi folks,
>>> 
>>> Looking for a review of this simple enough fix:
>>> 
>>> http://cr.openjdk.java.net/~robm/6947916/webrev.01/
>>> https://bugs.openjdk.java.net/browse/JDK-6947916
>> I think that the source changes are good, but the tests has a
>> reference to a shell script that is absent. Also, could the test just
>> create a simple jar file, rather than checking in a binary artifact.
>> 
>> -Chris.
>> 
>>> In a nutshell, if multiple URLConnections are made to several files in 
>>> a single jar, each will use the same backing JarFile object. 
>>> Unfortunately JarURLConnections connect() method recreates the 
>>> jarFileURLConnection for a given JarURLConnection using the default 
>>> value for getUseCaches instead of the *current* value.
>>> 
>>> In effect this means that jarURLConnection.getUseCaches() may return 
>>> true before calling connect() and false after. This in turn means that 
>>> when a JarURLConnection's inputstream is closed, it will believe that 
>>> caching has been turned off and the underlying jarFile will be closed 
>>> out from under all other JarURLConnection inputstreams.
>>> 
>>> -Rob
 

Re: RFR: 6947916: JarURLConnection does not handle useCaches correctly

[Rob McKenna]

To be explicit, new webrev here:

http://cr.openjdk.java.net/~robm/6947916/webrev.03/

-Rob

On 09/09/16 07:03, Rob McKenna wrote:
> Chris just pointed out to me that the test.classes prefix on the jar path is 
> unnecessary. He also mentioned that jtreg would clear up the scratch 
> directory so the deleteOnExit wouldn't be needed either.
> 
>   -Rob
> 
> On 09/09/16 05:02, Rob McKenna wrote:
> > Will do
> > 
> > -Rob
> > 
> > On 09/09/16 11:00, Roger Riggs wrote:
> > > Hi Rob,
> > > 
> > > Looks ok.
> > > 
> > > Its also a good practice to cleanup the file.  (File.deleteOnExit).
> > > 
> > > Roger
> > > 
> > > 
> > > On 9/9/2016 9:23 AM, Rob McKenna wrote:
> > > >Hey Chris,
> > > >
> > > >Apologies, I'm guilty of "just doing what adjacent tests do" here.
> > > >
> > > >That shell script is actually there in the test source already, but 
> > > >generating the jar from the test means theres no need to use it or to 
> > > >check in a binary. Thanks for picking me up!
> > > >
> > > >http://cr.openjdk.java.net/~robm/6947916/webrev.02/
> > > >
> > > > -Rob
> > > >
> > > >On 08/09/16 08:40, Chris Hegarty wrote:
> > > >>>On 7 Sep 2016, at 14:17, Rob McKenna  wrote:
> > > >>>
> > > >>>Hi folks,
> > > >>>
> > > >>>Looking for a review of this simple enough fix:
> > > >>>
> > > >>>http://cr.openjdk.java.net/~robm/6947916/webrev.01/
> > > >>>https://bugs.openjdk.java.net/browse/JDK-6947916
> > > >>I think that the source changes are good, but the tests has a
> > > >>reference to a shell script that is absent. Also, could the test just
> > > >>create a simple jar file, rather than checking in a binary artifact.
> > > >>
> > > >>-Chris.
> > > >>
> > > >>>In a nutshell, if multiple URLConnections are made to several files in 
> > > >>>a single jar, each will use the same backing JarFile object. 
> > > >>>Unfortunately JarURLConnections connect() method recreates the 
> > > >>>jarFileURLConnection for a given JarURLConnection using the default 
> > > >>>value for getUseCaches instead of the *current* value.
> > > >>>
> > > >>>In effect this means that jarURLConnection.getUseCaches() may return 
> > > >>>true before calling connect() and false after. This in turn means that 
> > > >>>when a JarURLConnection's inputstream is closed, it will believe that 
> > > >>>caching has been turned off and the underlying jarFile will be closed 
> > > >>>out from under all other JarURLConnection inputstreams.
> > > >>>
> > > >>>   -Rob
> > > 

Re: RFR: 6947916: JarURLConnection does not handle useCaches correctly

[Rob McKenna]

Chris just pointed out to me that the test.classes prefix on the jar path is 
unnecessary. He also mentioned that jtreg would clear up the scratch directory 
so the deleteOnExit wouldn't be needed either.

-Rob

On 09/09/16 05:02, Rob McKenna wrote:
> Will do
> 
>   -Rob
> 
> On 09/09/16 11:00, Roger Riggs wrote:
> > Hi Rob,
> > 
> > Looks ok.
> > 
> > Its also a good practice to cleanup the file.  (File.deleteOnExit).
> > 
> > Roger
> > 
> > 
> > On 9/9/2016 9:23 AM, Rob McKenna wrote:
> > >Hey Chris,
> > >
> > >Apologies, I'm guilty of "just doing what adjacent tests do" here.
> > >
> > >That shell script is actually there in the test source already, but 
> > >generating the jar from the test means theres no need to use it or to 
> > >check in a binary. Thanks for picking me up!
> > >
> > >http://cr.openjdk.java.net/~robm/6947916/webrev.02/
> > >
> > >   -Rob
> > >
> > >On 08/09/16 08:40, Chris Hegarty wrote:
> > >>>On 7 Sep 2016, at 14:17, Rob McKenna  wrote:
> > >>>
> > >>>Hi folks,
> > >>>
> > >>>Looking for a review of this simple enough fix:
> > >>>
> > >>>http://cr.openjdk.java.net/~robm/6947916/webrev.01/
> > >>>https://bugs.openjdk.java.net/browse/JDK-6947916
> > >>I think that the source changes are good, but the tests has a
> > >>reference to a shell script that is absent. Also, could the test just
> > >>create a simple jar file, rather than checking in a binary artifact.
> > >>
> > >>-Chris.
> > >>
> > >>>In a nutshell, if multiple URLConnections are made to several files in a 
> > >>>single jar, each will use the same backing JarFile object. Unfortunately 
> > >>>JarURLConnections connect() method recreates the jarFileURLConnection 
> > >>>for a given JarURLConnection using the default value for getUseCaches 
> > >>>instead of the *current* value.
> > >>>
> > >>>In effect this means that jarURLConnection.getUseCaches() may return 
> > >>>true before calling connect() and false after. This in turn means that 
> > >>>when a JarURLConnection's inputstream is closed, it will believe that 
> > >>>caching has been turned off and the underlying jarFile will be closed 
> > >>>out from under all other JarURLConnection inputstreams.
> > >>>
> > >>> -Rob
> > 

Re: RFR: 6947916: JarURLConnection does not handle useCaches correctly

[Rob McKenna]

Will do

-Rob

On 09/09/16 11:00, Roger Riggs wrote:
> Hi Rob,
> 
> Looks ok.
> 
> Its also a good practice to cleanup the file.  (File.deleteOnExit).
> 
> Roger
> 
> 
> On 9/9/2016 9:23 AM, Rob McKenna wrote:
> >Hey Chris,
> >
> >Apologies, I'm guilty of "just doing what adjacent tests do" here.
> >
> >That shell script is actually there in the test source already, but 
> >generating the jar from the test means theres no need to use it or to check 
> >in a binary. Thanks for picking me up!
> >
> >http://cr.openjdk.java.net/~robm/6947916/webrev.02/
> >
> > -Rob
> >
> >On 08/09/16 08:40, Chris Hegarty wrote:
> >>>On 7 Sep 2016, at 14:17, Rob McKenna  wrote:
> >>>
> >>>Hi folks,
> >>>
> >>>Looking for a review of this simple enough fix:
> >>>
> >>>http://cr.openjdk.java.net/~robm/6947916/webrev.01/
> >>>https://bugs.openjdk.java.net/browse/JDK-6947916
> >>I think that the source changes are good, but the tests has a
> >>reference to a shell script that is absent. Also, could the test just
> >>create a simple jar file, rather than checking in a binary artifact.
> >>
> >>-Chris.
> >>
> >>>In a nutshell, if multiple URLConnections are made to several files in a 
> >>>single jar, each will use the same backing JarFile object. Unfortunately 
> >>>JarURLConnections connect() method recreates the jarFileURLConnection for 
> >>>a given JarURLConnection using the default value for getUseCaches instead 
> >>>of the *current* value.
> >>>
> >>>In effect this means that jarURLConnection.getUseCaches() may return true 
> >>>before calling connect() and false after. This in turn means that when a 
> >>>JarURLConnection's inputstream is closed, it will believe that caching has 
> >>>been turned off and the underlying jarFile will be closed out from under 
> >>>all other JarURLConnection inputstreams.
> >>>
> >>>   -Rob
> 

Re: RFR: 6947916: JarURLConnection does not handle useCaches correctly

[Roger Riggs]

Hi Rob,

Looks ok.

Its also a good practice to cleanup the file.  (File.deleteOnExit).

Roger


On 9/9/2016 9:23 AM, Rob McKenna wrote:

Hey Chris,

Apologies, I'm guilty of "just doing what adjacent tests do" here.

That shell script is actually there in the test source already, but generating 
the jar from the test means theres no need to use it or to check in a binary. 
Thanks for picking me up!

http://cr.openjdk.java.net/~robm/6947916/webrev.02/

-Rob

On 08/09/16 08:40, Chris Hegarty wrote:

On 7 Sep 2016, at 14:17, Rob McKenna  wrote:

Hi folks,

Looking for a review of this simple enough fix:

http://cr.openjdk.java.net/~robm/6947916/webrev.01/
https://bugs.openjdk.java.net/browse/JDK-6947916

I think that the source changes are good, but the tests has a
reference to a shell script that is absent. Also, could the test just
create a simple jar file, rather than checking in a binary artifact.

-Chris.


In a nutshell, if multiple URLConnections are made to several files in a single 
jar, each will use the same backing JarFile object. Unfortunately 
JarURLConnections connect() method recreates the jarFileURLConnection for a 
given JarURLConnection using the default value for getUseCaches instead of the 
*current* value.

In effect this means that jarURLConnection.getUseCaches() may return true 
before calling connect() and false after. This in turn means that when a 
JarURLConnection's inputstream is closed, it will believe that caching has been 
turned off and the underlying jarFile will be closed out from under all other 
JarURLConnection inputstreams.

-Rob

Re: RFR: 6947916: JarURLConnection does not handle useCaches correctly

[Chris Hegarty]

> On 7 Sep 2016, at 14:17, Rob McKenna  wrote:
> 
> Hi folks,
> 
> Looking for a review of this simple enough fix:
> 
> http://cr.openjdk.java.net/~robm/6947916/webrev.01/
> https://bugs.openjdk.java.net/browse/JDK-6947916

I think that the source changes are good, but the tests has a
reference to a shell script that is absent. Also, could the test just
create a simple jar file, rather than checking in a binary artifact.

-Chris.

> In a nutshell, if multiple URLConnections are made to several files in a 
> single jar, each will use the same backing JarFile object. Unfortunately 
> JarURLConnections connect() method recreates the jarFileURLConnection for a 
> given JarURLConnection using the default value for getUseCaches instead of 
> the *current* value.
> 
> In effect this means that jarURLConnection.getUseCaches() may return true 
> before calling connect() and false after. This in turn means that when a 
> JarURLConnection's inputstream is closed, it will believe that caching has 
> been turned off and the underlying jarFile will be closed out from under all 
> other JarURLConnection inputstreams.
> 
>   -Rob