On Fri, 7 Apr 2023 14:16:32 GMT, Archie L. Cobbs <d...@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

Reply via email to