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

[Seán Coffey]

On 10/04/2014 11:49, Pavel Rappo wrote:
> 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.
It's a fair point Pavel. All java.util.zip classes would need to be 
looked at to claim that zip package would be thread-safe. It still only 
makes sense to have only one thread operating on such streams and if the 
application does code to that expectation maybe the synchronization 
overhead is not too costly.

Maybe an enhancement request could be opened to look into it. I'd like 
to backport the below change to 8u, 7u and keep that separate.

regards,
Sean.
>
> -Pavel
>
> On 10 Apr 2014, at 11:35, Seán Coffey <sean.coffey at 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.