RFR 8080225 FileInput/OutputStream/FileChannel cleanup should be improved

[Roger Riggs]

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