RFR: 8175010: ImageReader is not thread-safe

[Claes Redestad]

Jim, Chris, Alan, thanks for reviewing!

On 02/15/2017 02:56 PM, Alan Bateman wrote:
> On 15/02/2017 13:22, Claes Redestad wrote:
>
>> Hi,
>>
>> a few intermittent but rare test failures[1] that has appeared
>> since the latest jake integration, and since one of the changes
>> in there was to make initialization of the system ImageReader
>> lazy there appears to be cases where ImageReaders are not
>> safely published:
>>
>> - Ensure ImageReader::open is called only once per Path in
>> ImageReaderFactory by using CHM.computeIfAbsent
>> - Ensure ImageReader.reader is safely published to a
>> final field and signal close using a volatile boolean instead
>>
>> webrev: http://cr.openjdk.java.net/~redestad/8175010/webrev.02/
>> bug: https://bugs.openjdk.java.net/browse/JDK-8175010
> ImageReaderFactory looks good.
>
> The changes to ImageReader are okay too, always a bit odd that this 
> code throw NPE when the reader was closed. There is still an issue 
> with async close of course in that someone could close at the same 
> time as an access. However that is a high-level issue for jrtfs, at 
> run-time then the image file is opened once and is never closed.

Yes, this patch doesn't attempt to improve on how races are handled when 
closing, but should help to avoid potential publication issues when opening.

It seems it could be worthwhile to have a special implementation class 
for the system module, since we could make it perfectly shareable and 
non-closeable, thus avoid delegation and closed-checking overheads, but 
that'd be a larger endeavor, perhaps suitable as an RFE for 10.

/Claes

>
> -Alan
>
>