8025619: (fc) FileInputStream.getChannel on closed stream returns FileChannel that doesn't know that stream is closed

[Brian Burkhalter]

On Dec 9, 2014, at 12:14 AM, Alan Bateman <Alan.Bateman at oracle.com> wrote:

> This looks better except for getChannel which isn't right in this iteation. I think it's best to change this to use the double-checked locking idiom, this will give something like:
> 
> FileChannel fc = this.channel;
> if (fc == null) {
>     synchornized (this) {
>         fc = this.channel;
>         if (fc == null) {
>             this.channel = fc = FileChannelImpl.open(...);
>             :
>         }
>     }
> }
> return fc;

I was using the form suggested in this article

http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html

just before the references. Modified.

> A minor comment but you can drop the initializing of channel to null (to avoid a needless volatile write).

Modified.

> In close then you can eliminate a volatile-read with FileChannel fc = channel; if (fc != null) fc.close();

Modified.

> In the test then it would be good to assert !fc.isOpen();

Added.

> The tryLock is still a bit odd,

That was originally to test the change in FileKey.create(). While this change is not strictly necessary I think it is appropriate.

> shouldn't that be checking for ClosedChannelException too?

Modified.

> An alternative name for the test is GetClosedChannel if you are looking for another name.

Name changed.

Here’s an updated patch:

http://cr.openjdk.java.net/~bpb/8025619/webrev.03/

Thanks,

Brian
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/nio-dev/attachments/20141209/81921ffb/attachment-0001.html>