On 4/8/14 10:29 AM, Seán Coffey wrote:
Sherman,
I see rem == 0 condition becoming true before the zentry variable is
set to 0 (in close()) - in a multi threaded scenario like this one, we
could have zero remaining bytes in zentry and yet have a zentry != 0 -
your suggestion would prevent the SEGV (since we're in the sync block)
but we would needlessly cycle into the ensureOpenOrZipException() and
ZipFile.read code (line 713) if another thread tasked with the close()
call was knocked off by the dispatcher during the various rem == 0
checks that we make : e.g
722 if (rem == 0) {
723 close();
724 }
Am I reading this correctly ?
My take is that the performance is not a concern here, the only real
problem is the SEGV.
Given "num" is not a volatile and is not updated under synchronized
block, check "num == 0"
is not going to make ZFIS work for mult-thread usage. It also makes me
nervous to check it
inside the synchronized block as a global "flag". I'm also concerned
that the change to check
the rem == 0 after the check of "len" may also change the behavior of
someone's code in
certain circumstance...
-Sherman
regards,
Sean.
On 08/04/2014 16:59, Xueming Shen wrote:
Hi Sean,
It might be more explicit to check "if (zentry == 0)" here?
-Sherman
On 4/8/14 8:21 AM, Seán Coffey wrote:
A recently reported bug shows a race condition is possible on the
rem == 0 check in ZipFile.read(byte b[], int off, int len). A bad
check can result in referencing a jzentry structure that might
already be freed and hence result in a SEGV. The fix is trivial and
involves moving the rem == 0 check into a synchronized block. The
ZipFile API itself is not thread safe so having mutiple threads
operate on the same ZipFileInputStream is something that should
never be performed.
webrev : http://cr.openjdk.java.net/~coffeys/webrev.8038491/webrev/
regards,
Sean.
