RFR 8080225 FileInput/OutputStream/FileChannel cleanup should be improved (original) (raw)

mandy chung mandy.chung at oracle.com
Tue Dec 5 19:27:10 UTC 2017

On 12/1/17 6:38 PM, Roger Riggs wrote:

Please review a revision to the work on remove a dependency on finalization from FileInputStream, FileOutputStream, and add cleanup of closing file descriptors in FileChannel.

This revised approach sounds reasonable that minimizes the compatibility risk in JDK 10 while giving an advanced notice to existing applications to prepare for the removal of FIS/FOS finalizer method and existing subclasses that override the close method should make appropriate change to prepare the incompatibility risk when the FIS/FOS finalizer is removed in a future release.

Thanks for doing this.

Webrev: http://cr.openjdk.java.net/~rriggs/webrev-fis-cleanup-8080225/

Looks good overall.   I agree with Peter's suggestion that subclasses overriding the close method will use AltFinalizer and subclasses overriding the finalizer but not close method will get invoked anyway and it can employ the cleaner to close the stream in that case.  I see the webrev has been updated to reflect that.

Minor comments:

Nit: I wonder if {@link #close} should be used instead of {@linkplain #close} in the javadoc (as I read that referring to the method rather than the action).   Just to mention it.

The javadoc of the close method:

343 * @implNote 344 * Overriding {@linkplain #close} to perform cleanup actions is reliable 345 * only when called directly or when called by try-with-resources.

I think this can be @apiNote, like the @apiNote added in the finalize method.  Maybe you want to promote this to @apiNote when the finalize method is removed.

test/jdk/sun/security/provider/FileInputStreamPool/FileInputStreamPoolTest.java Nit: @modules would be good to keep it sorted. Formatting nit: line 84-106: it reads to me that the indentation is 5-space

thanks Mandy

More information about the core-libs-dev mailing list