RFR : 8038491: Improve synchronization in ZipFile.read()

Chris Hegarty chris.hegarty at oracle.com
Wed Apr 9 20:56:05 UTC 2014


> On 9 Apr 2014, at 20:10, Seán Coffey <sean.coffey at oracle.com> 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.
> 



More information about the core-libs-dev mailing list