Morning Christoph,
> On Feb 21, 2020, at 3:11 AM, Langer, Christoph <[email protected]> > 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 <[email protected] > <mailto:[email protected]>> > Sent: Donnerstag, 20. Februar 2020 20:00 > To: Langer, Christoph <[email protected] > <mailto:[email protected]>> > Cc: [email protected] <mailto:[email protected]>; > [email protected] <mailto:[email protected]> > 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 <[email protected] > <mailto:[email protected]>> 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 > [email protected] <mailto:[email protected]> > > > > <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 [email protected] <mailto:[email protected]>
