RFR: 8305748: Clarify reentrant behavior of close() in FileInputStream, FileOutputStream, and RandomAccessFile [v2]

[Alan Bateman]

On Fri, 7 Apr 2023 14:16:32 GMT, Archie L. Cobbs <duke at openjdk.org> wrote:

>> IO stream classes like `FileOutputStream` can have assocated NIO channels.
>> 
>> When `close()` is invoked on one of these classes, it in turn invokes `close()` on the associated channel (if any). But when the associated channel's `close()` method is invoked, it in turn invokes `close()` on the associated stream (if any).
>> 
>> As a result, these IO stream `close()` methods invoke themselves reentrantly when there is an associated channel.
>> 
>> This is not a problem for these classes because they are written to handle this (i.e., they are idempotent), but it can be surprising (or worse, just silently bug-inducing) for subclasses that override `close()`.
>> 
>> There are two possible ways to address this:
>> 1. Modify the code to detect and avoid the (unnecessary) reentrant invocations
>> 2. Add a `@implNote` to the Javadoc so subclass implementers are made aware
>> 
>> This patch takes the second, more conservative approach.
>
> Archie L. Cobbs has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Apply Javadoc improvements suggested in review.

src/java.base/share/classes/java/io/FileInputStream.java line 503:

> 501:      *
> 502:      * @apiNote
> 503:      * When this stream has an associated channel, this method may invoke

I don't think a method can have more than one apiNote so the existing note will run into this text. Replacing @apiNote with a paragraph tag will help.

As regards the wording, then here's a suggestion that relies on the specification and avoids giving the impression that the close method directly calls itself recursively:

"If this stream has an associated channel then this method will close the channel, which in turn will close this stream. Subclasses that override this method should be prepared to handle possible reentrant invocation."

src/java.base/share/classes/java/io/RandomAccessFile.java line 701:

> 699:      *
> 700:      * @apiNote
> 701:      * When this file has an associated channel, this method may invoke

"this file" should be "this stream"

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/13379#discussion_r1160864473
PR Review Comment: https://git.openjdk.org/jdk/pull/13379#discussion_r1160864654