RFR 8239556: (zipfs) remove ExistingChannelCloser facility in zipfs implementation

[Lance Andersen]

Morning Christoph,


> On Feb 21, 2020, at 3:11 AM, Langer, Christoph <christoph.langer at sap.com> wrote:
> 
> Hi Lance,
>  
> thanks for reviewing. Any results from Mach5? I’ll push it after I get your ok.

It finished late last night and it ran clean
>  
> The manifest fix will still apply after this – but you wanted to update the webrev anyway with some test updates, right?
Yes, I wanted to clean up the test some.
>  
> When both of these fixes are in, I’ll look at augmenting Jai’s current proposal for JDK-8225507 to overhaul exception handling in close()&sync().

Assume your last webrev on top of this fix is the lucky winner while you take a stab at this clean up :-)

Best
Lance
>  
> Cheers
> Christoph
>  
>  
>  
> From: Lance Andersen <lance.andersen at oracle.com <mailto:lance.andersen at oracle.com>> 
> Sent: Donnerstag, 20. Februar 2020 20:00
> To: Langer, Christoph <christoph.langer at sap.com <mailto:christoph.langer at sap.com>>
> Cc: nio-dev at openjdk.java.net <mailto:nio-dev at openjdk.java.net>; core-libs-dev at openjdk.java.net <mailto:core-libs-dev at openjdk.java.net>
> Subject: Re: RFR 8239556: (zipfs) remove ExistingChannelCloser facility in zipfs implementation
>  
> Hi Christoph,
>  
> This looks good and thank you for tackling. We will need to regenerate the web-rev for the Manifest fix after this gets pushed
>  
> I am running Mach5 tiers 1-3 now and will let you know when it completes
>  
>  
> 
> 
> On Feb 20, 2020, at 8:33 AM, Langer, Christoph <christoph.langer at sap.com <mailto:christoph.langer at sap.com>> wrote:
>  
> Hi,
> 
> please review this cleanup change to remove the ExistingChannelCloser facility in zipfs.
> 
> Some technical details about why this existed can be found in this mailing list thread, where Sherman explained its original purpose: https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-April/059839.html <https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-April/059839.html>. At that time we didn't dare to remove it and deferred to further investigation.
> 
> Now, when looking at the close() implementation of ZipFileSystem during review of JDK-8225507, I had a closer look to exChannelCloser as well and found that it should definitely be removed. Its original purpose was to keep a backing file and channel for InputStreams that were still open while the sync() method was called to update a zip file on disk. However, in its current state, a zip file is only synced to disk by sync() during close(), at a time when all InputStreams should already be closed.
> 
> Closing of the streams is done at the beginning of method close(), in line 466ff. Closing of any open InputStream would remove it from the list named "streams". After that, it must be assured that no InputStreams get created, of course. This is accomplished by calling ensureOpen() under readLock, whenever an InputStream shall be created. The first step of close() though is to set the ZipFileSystem to state "closed", so these checks should all fail. I have, however, found one location where this ensureOpen check was missing and added it.
> 
> jdk/nio/zipfs tests still pass.
> 
> Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8239556.0/ <http://cr.openjdk.java.net/~clanger/webrevs/8239556.0/>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8239556 <https://bugs.openjdk.java.net/browse/JDK-8239556>
> 
> Thanks
> Christoph
> 
>  
> <image001.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
> 
>  <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
> Oracle Java Engineering 
> 1 Network Drive 
> Burlington, MA 01803
> Lance.Andersen at oracle.com <mailto:Lance.Andersen at oracle.com>
>  
> 
> 
>  

 <http://oracle.com/us/design/oracle-email-sig-198324.gif>
 <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
 <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
Lance.Andersen at oracle.com <mailto:Lance.Andersen at oracle.com>