zipfs problems

[Joel Uckelman]

Thus spake Alan Bateman:
> Joel Uckelman wrote:
> > Hi,
> >
> > I've been looking at the zipfs demo and have noticed some problems:
> >
> > * The HashSet ZipFileSystem.closeableObjects could be modified from two
> >   different threads if each thread calls ZipFilePath.newInputStream() or
> >   newByteChannel() at about the same time.
> >
> > * Closeables are only removed from ZipFileSystem.closeableObjects when
> >   the ZipFileSystem is closed. This means that closed streams and channels
> >   will stick around long after they could otherwise be gc'd.
> >
> > * ZipFileSystemProvider.fileSystems has concurrency problems similar to
> >   ZipFileSystem.closeableObjects.
> >   
> 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.

I'll be sending in the contributor agreement when I get to the office 
tomorrow (I can't print at home), so I'll be happy to start sumitting
patches then.
 
> > * Should ZipFileSystemProvider.newFileSystem() really return two distinct
> >   FileSystem objects when called with two different env Maps?
> >   
> I don't see a problem here as the newFileSystem methods are required to 
> return a new file system (irrespective of the entries in the map). Note 
> that it's okay for several FileSystem instances to share access to the 
> underlying zip file and this can be transparent to the user of the API.

I'm viewing this with the intention of adding the ability to write ZIP
archives also. Having multiple FileSystem instances for the same archive
and doing some writes with each one might lead to trouble. OTOH, maybe
the place for ensuring that the state of the archive remains consistent
is ZipFileStore---every FileSystem for the same archive could share one
ZipFileStore.

It's not clear to me what should happen if someone tries to open multiple
streams or channels on the same archive entry. With a disk filesystem,
there's nothing stopping you from opening a FileInputStream and a
FileOutputStream on the same file at the same time. In some limited cases,
it might not even be wrong, e.g., if you're reading from and writing to
non-overlapping parts of a file. Various other pathological things are
possible...

If you close() a read-write archive, that should cause whatever writing
was done (by writing to the temporary file cache) to be written back to
the archive---which could mean that if there are other ZipFileSystem
instances for the same archive, then the archive will have changed out
from under them. Whether this is a problem, I'm unsure at present. Maybe
preventing this should just be the responsibility of the user? Hmm.

-- 
J.