RFR 8080225 FileInput/OutputStream/FileChannel cleanup should be improved

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