> On 9 Apr 2014, at 20:10, Seán Coffey <[email protected]> wrote: >> On 09/04/14 19:36, Alan Bateman wrote: >>> On 09/04/2014 18:39, Seán Coffey wrote: >>> On re-read, I believe extending the sync block in read(..) to cover the >>> reading and setting of the rem variable works to resolve this fix. It >>> should preserve behaviour as well. >>> >>> http://cr.openjdk.java.net/~coffeys/webrev.8038491.v2/webrev/ >> This is probably okay for a test case that attempts concurrent reads but as >> it's not synchronized with skip then I don't think you can trust the value >> of rem or pos without taking copies and validating. > 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.
Right. This is still NOT thread-safe, but just enough to "support" async close and prevent the potential of a crash. Maybe a comment would help, or maybe not. -Chris. > Let me know if I need to make changes. I don't think we want to add extra > sync costs to the class. > > regards, > Sean. > >> >> -Alan. >
