RFR: JDK-8225507: (zipfs): No space left on device not thrown with ZipFileSystem

[Langer, Christoph]

Hi,

I just had a look at the implementations of close() and sync() and from what I can see there should probably be a lot more done to make sure the ZipFileSystem instance is in a closed and clean state when an IOException was encountered along the path of writing it.

In sync, we should probably move some stuff to a finally block (e.g. closing of the channel, make sure the tmpFile is removed etc.). I'm also not sure about the exChClosers. I don't fully understand yet what this is needed for. I guess this should be examined, too.

In close(), there are probably also quite some steps that could be moved to a finally block, even if sync() came back with an Exception.

Let me see if I find the time to come up with some prototype soon...

But overall, I agree, these IOExceptions definitely shouldn't be swallowed and rather be propagated to the caller.

Best regards
Christoph

From: nio-dev <nio-dev-bounces at openjdk.java.net> On Behalf Of Lance Andersen
Sent: Mittwoch, 12. Februar 2020 23:22
To: Alan Bateman <Alan.Bateman at oracle.com>
Cc: nio-dev at openjdk.java.net
Subject: Re: RFR: JDK-8225507: (zipfs): No space left on device not thrown with ZipFileSystem

A few observations from looking at:

close method:


  *   Any open InputStreams are closed so there is a possibility of an IOException being thrown
  *   The channel is closed in the same try block that sync() is invoked so there is a potential that if sync() fails the channel is not closed cleanly
  *   the removal of the filesystem for URI based invocations of newFileSystem should/could be moved up after the Zip FS is moved closed
  *   I suspect we should defer the throwing of the IOException if sync() fails until after we finish the rest of close() similar as is done for an IOException occurring while deleting the temp filess

sync method:


  *   Looks like if the write of a CEN entry fails the IOException would be thrown where as an issue updating the LOC will just be ignored which I am not sure why that was done


Best
Lance


On Feb 9, 2020, at 12:02 PM, Alan Bateman <Alan.Bateman at oracle.com<mailto:Alan.Bateman at oracle.com>> wrote:

On 05/02/2020 23:34, Lance Andersen wrote:

Hi Jaikiran,

I think overall the patch is correct but it would be good to get some extra eyes on this.
On the surface this looks okay but I think it would be useful if you (or Jai) could write a brief summary about the state of the file system and the underlying channel when the sync fails. I think the zip file system state will be closed (!isOpen) so a later attempt to close it will be a no-op. However, the state of the channel is not clear - is it open or closed?

-Alan

[cid:image001.gif at 01D5E715.1DDB2BD0]<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>




-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/nio-dev/attachments/20200219/dd6d538c/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: image001.gif
Type: image/gif
Size: 658 bytes
Desc: image001.gif
URL: <https://mail.openjdk.java.net/pipermail/nio-dev/attachments/20200219/dd6d538c/image001.gif>