Hi Roger,
This looks good overall.
53 * unreachable should explicitly override {@link #finalize} and call
{@code close}.
Since finalize is deprecated, I would not recommend to have the subclass
adding the finalize method. I suggest to take that phrase out (I gave a
similar comment to JDK-8185582 [1]). Users should use
try-with-resource or register the instances in a cleaner.
As for the tests, FinalizeShdCallClose.java implements finalize. I
think it'd be good to convert them to try-with-resource.
I'm not close to java.io implementation. Would registerCleanup be
called more than once? line 220 in the registerCleanup will create a new
cleanup if it's invoked the second time - is it intentional?
216 synchronized void registerCleanup() {
217 if (cleanup != null) {
218 cleanup.clear();
219 }
220 cleanup = FDCleanup.create(this);
221 } thanks Mandy
[1]
http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-September/049362.html
On 9/29/17 10:17 AM, Roger Riggs wrote:
Replacing finalizers in FileInputStream, FileOutputStream, and adding
cleanup to RandomAccessFile
and NIO File Channels with Cleaner based implementations required
changes to FileDescriptor.
The specification of FileInputStream and FileOutputStream is changed
to remove the finalizer
behavior that required their respective close methods to be called.
This change to the behavior is tracked with CSR 8187325 [3].
The FileDescriptor implementations (Unix and Windows) provide a
cleanup function that is now used by
FIS, FOS, RAF, and async and normal FileChannels. Each requests
FileDescriptor to register a cleanup function
when the fd or handle is assigned and delegates to
FileDescriptor.close. If the respective
FileDescriptor.close method is not called, the fd or handle will be
closed when the FileDescriptor
is determined to be phantom reachable.
The other uses of FileDescriptor are not intended to be changed, for
example in sockets and datagrams.
Some tests were modified to not rely on finalization; new tests are
provided.
Comments are appreciated on both the CSR [3] and the implementation [1].
[1] webrev:
http://cr.openjdk.java.net/~rriggs/webrev-fis-cleanup-8080225/
[2] Issue: https://bugs.openjdk.java.net/browse/JDK-8080225
[3] CSR: 8187325 FileInputStream/FileOutputStream should use the
Cleaner instead of finalize
https://bugs.openjdk.java.net/browse/JDK-8187325
Thanks, Roger
