On Thu, 6 May 2021 07:41:00 GMT, Alan Bateman <al...@openjdk.org> wrote:
>> Jason Zaugg has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Remove redundant cast from previous commit.
>
> src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java line 1235:
>
>> 1233: synchronized(ch) {
>> 1234: return ch.position(pos).read(bb);
>> 1235: }
>
> I think it's okay to include the update to EntryInputStream, that part looks
> fine, as does the directly use of the FileChannel positional read.
>
> I'm still mulling over the case where ch is not a FileChannel as I would
> expected it to capture the existing position and restore it after the read. I
> think this is the degenerative case when the zip file is located in a custom
> file system that doesn't support FileChannel. In that case, positional read
> has to be implemented on the most basic SeekableByteChannel. It would only be
> observed when mixing positional read ops with other ops that depend on the
> current position.
Here are all the references to `ch`.
this.ch = Files.newByteChannel(zfpath, READ);
...
this.ch.close();
...
ch.close(); // close the ch just in case no update
...
if (ch instanceof FileChannel fch) {
return fch.read(bb, pos);
} else {
synchronized(ch) {
return ch.position(pos).read(bb);
}
}
...
long ziplen = ch.size();
...
ch.close();
It appears the only position-dependent operation called `read(ByteBuffer)`.
This is performed together with the `pos` call within the `synchronized(ch)`
lock.
-------------
PR: https://git.openjdk.java.net/jdk/pull/3853
