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

Langer, Christoph christoph.langer at sap.com
Fri Feb 21 08:11:16 UTC 2020


Hi Lance,

thanks for reviewing. Any results from Mach5? I'll push it after I get your ok.

The manifest fix will still apply after this - but you wanted to update the webrev anyway with some test updates, right?

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().

Cheers
Christoph



From: Lance Andersen <lance.andersen at oracle.com>
Sent: Donnerstag, 20. Februar 2020 20:00
To: Langer, Christoph <christoph.langer at sap.com>
Cc: nio-dev at openjdk.java.net; 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. 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/
Bug: https://bugs.openjdk.java.net/browse/JDK-8239556

Thanks
Christoph

[cid:image001.gif at 01D5E896.DE3E9740]<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>






More information about the core-libs-dev mailing list