Hi Mandy,
Updated the webrev in place:
http://cr.openjdk.java.net/~rriggs/webrev-fis-cleanup-8080225/
On 12/5/2017 2:27 PM, mandy chung wrote:
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.
Thanks for confirming
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.
done
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.
Yes, it is more like an apiNote
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
fixed
Thanks, Roger
