zipfs problems

[Joel Uckelman]

Thus spake Joel Uckelman:
> Thus spake Alan Bateman:
> >   
> > You are right and there are concurrency issues here. Also all returned 
> > closeables should be wrapped so that close method removes them from the 
> > set.  Rajendra, do you have cycles to fix this?
> 
> Actually, I've already done it for closeableObjects. The only issue I'm
> wondering aobut is whether to use Collections.synchronizedSet() or to
> build a Set wrapper for a ConcurrentHashMap.
> 

Here's a patch for this issue.

-------------- next part --------------
# HG changeset patch
# User uckelman at adsl-208-39.dsl.uva.nl
# Date 1256066426 -7200
# Node ID 707295cce2384eb8083246a7910b862f997bfa8d
# Parent  9b426af16c0dae4b85a53e9766c6fe92345a31a5
* Renamed closeableObjects to closeables (Objects is redundant).
* closeables is now a synchronized Set to prevent concurrent modification
* InputStreams and SeekableByteChannels now register themselves with
  ZipFileSystem on creation and unregister themselves on closure.

diff --git a/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipFilePath.java b/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipFilePath.java
--- a/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipFilePath.java
+++ b/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipFilePath.java
@@ -31,6 +31,7 @@
 package com.sun.nio.zipfs;
 
 import java.io.File;
+import java.io.FilterInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
@@ -727,6 +728,73 @@
         return pathToZip;
     }
 
+    /**
+     * An {@link InputStream} which registers itself with the
+     * {@link ZipFileSystem} on creation and unregisters itself on
+     * closure.
+     */
+    private class ZipFilePathInputStream extends FilterInputStream {
+        public ZipFilePathInputStream(InputStream in) {
+            super(in);
+            fileSystem.registerCloseable(this);
+        }
+
+        @Override
+        public void close() throws IOException {
+            in.close();
+            fileSystem.unregisterCloseable(this);
+        }
+    }
+
+    /**
+     * A {@link SeekableByteChannel} which registers itself with the
+     * {@link ZipFileSystem} on creation and unregisters itself on
+     * closure.
+     */
+    private class ZipFilePathChannel implements SeekableByteChannel {
+        private final SeekableByteChannel ch;
+
+        public ZipFilePathChannel(SeekableByteChannel ch) {
+            this.ch = ch;
+            fileSystem.registerCloseable(this);
+        }
+
+        public long position() throws IOException {
+            return ch.position();
+        }
+
+        public SeekableByteChannel position(long newPos) throws IOException {
+            ch.position(newPos);
+            return this;
+        }
+    
+        public int read(ByteBuffer dst) throws IOException {
+            return ch.read(dst);
+        }
+    
+        public long size() throws IOException {
+            return ch.size();
+        }
+
+        public SeekableByteChannel truncate(long size) throws IOException {
+            ch.truncate(size);
+            return this;
+        }
+
+        public int write(ByteBuffer src) throws IOException {
+            return ch.write(src);
+        }
+
+        public boolean isOpen() {
+            return ch.isOpen();
+        }
+
+        public void close() throws IOException {
+            ch.close();
+            fileSystem.unregisterCloseable(this);
+        }
+    }
+
     @Override
     public InputStream newInputStream(OpenOption... options)
             throws IOException {
@@ -751,9 +819,8 @@
                     zfile.close();
                     throw new IOException("entry not found" + entryStr);
                 }
-                InputStream is = zfile.getInputStream(entry);
-                fileSystem.addCloseableObjects(is);
-                return is;
+
+                return new ZipFilePathInputStream(zfile.getInputStream(entry));
             }
         } finally {
             end();
@@ -1009,9 +1076,10 @@
                 InputStream in = zfile.getInputStream(entry);
                 Path pathtoZip = Paths.get(ZipUtils.readFileInZip(in));
                 zfile.close();
-                SeekableByteChannel sbc = FileChannel.open(pathtoZip, options);
-                fileSystem.addCloseableObjects(sbc);
-                return sbc;
+
+                return new ZipFilePathChannel(
+                  FileChannel.open(pathtoZip, options)
+                );
             }
         } finally {
             end();
diff --git a/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipFileSystem.java b/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipFileSystem.java
--- a/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipFileSystem.java
+++ b/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipFileSystem.java
@@ -51,7 +51,9 @@
     private final String defaultdir;
     private final ReadWriteLock closeLock = new ReentrantReadWriteLock();
     private boolean open = true;
-    private Set<Closeable> closeableObjects = new HashSet<Closeable>();
+
+    private final Set<Closeable> closeables =
+      Collections.synchronizedSet(new HashSet<Closeable>());
 
     ZipFileSystem(ZipFileSystemProvider provider,
             FileRef fref) {
@@ -117,7 +119,7 @@
     private void implClose(URI root) throws IOException {
         ZipUtils.remove(root); // remove cached filesystem
         provider.removeFileSystem(root);
-        Iterator<Closeable> itr = closeableObjects.iterator();
+        Iterator<Closeable> itr = closeables.iterator();
         while (itr.hasNext()) {
             try {
                 itr.next().close();
@@ -128,8 +130,12 @@
         }
     }
 
-    boolean addCloseableObjects(Closeable obj) {
-        return closeableObjects.add(obj);
+    boolean registerCloseable(Closeable c) {
+        return closeables.add(c);
+    }
+
+    boolean unregisterCloseable(Closeable c) {
+        return closeables.remove(c);
     }
 
     @Override
# HG changeset patch
# User uckelman at adsl-208-39.dsl.uva.nl
# Date 1256067699 -7200
# Node ID 716fe8ce64d09f69265c46466f5d53eca296a35a
# Parent  707295cce2384eb8083246a7910b862f997bfa8d
Added missing import.

diff --git a/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipFilePath.java b/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipFilePath.java
--- a/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipFilePath.java
+++ b/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipFilePath.java
@@ -35,6 +35,7 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.nio.ByteBuffer;
 import java.nio.channels.SeekableByteChannel;
 import java.nio.file.*;
 import java.nio.file.DirectoryStream.Filter;