Given the amount of mutual exclusion and synchronization already involved, I wonder if it's worth making it thread-safe after all. And (idialy) forget about races there forever.
-Pavel On 10 Apr 2014, at 11:35, Seán Coffey <sean.cof...@oracle.com> wrote: > This should make the code more robust Alan. > > http://cr.openjdk.java.net/~coffeys/webrev.8038491.v3/webrev/ > > regards, > Sean. > > On 09/04/2014 22:16, Alan Bateman wrote: >> On 09/04/2014 20:10, Seán Coffey wrote: >>> >>> I played around with adding some skip testing Alan but didn't see it >>> increase the rate of failure on an unpatched binary. Since >>> ZipFileInputStream.read(..) is the trouble method and now has better >>> synchronized access to reading and updating rem, I believe we're good. >>> skip(long) can trigger a close() but close() can't free up the resources >>> without the ZipFile.this lock. As mentioned, having multiples threads >>> reading from the one zip stream would make no sense anyhow. >>> >>> Let me know if I need to make changes. I don't think we want to add extra >>> sync costs to the class. >> If you are only going to synchronize read then you will need to take a copy >> of rem and pos, otherwise you have to assume that the check and limiting of >> len isn't reliable, same thing for pos which could be way out of range due >> to concurrent usages of skip. >> >> -Alan. >