RFR: JDK-8080225: FileInputStream cleanup should be improved

[Peter Levart]

Hi,

As part of JEP 132 (More-prompt finalization) I wanted to find out what 
it would take to convert finalization-based cleanup code used in a JDK 
class to PhantomReference-based.

In FileInputStream case, the state needed for clean-up is stored in a 
FileDescriptor referenced from FileInputStream. To make things 
complicated, FileDescriptor instance can be shared among 
FileInputStream(s), FileOutputStream(s) and/or RandomAccessFile(s). 
Shared FileDescriptor instance keeps track (using strong references) of 
any FileInputStream, FileOutputStream or RandomAccessFile it is 
registered with (FileDescriptor.attach(Closeable)). A graph of 
FileInputStream(s), FileOutputStream(s) and/or RandomAccessFile(s) 
sharing a FileDescriptor is therefore strongly connected in all 
directions. FileInputStream and FileOutputStream override finalize() 
method, RandomAccessFile doesn't. When all FileInputStream(s), 
FileOutputStream(s) and/or RandomAccessFile(s) connected together with 
the shared FileDescriptor become eligible for finalization, finalize() 
methods of FileInputStream(s) and FileOutputStream(s) get called. They 
delegate to stream close() method(s) in turn which delegate to 
FileDescriptor.closeAll() which close()s all streams sharing the 
FileDescriptor and finally calls-back into the 1st stream which invoked 
closeAll() to close the FileDescriptor (via the call to native 
[FileInputStream|FileOutputStream|RandomAccessFile].close0() method).

To untangle this knot, I had to refactor some internal logic:

http://cr.openjdk.java.net/~plevart/jdk9-dev/8080225_FileStreamCleanUp/webrev.01/

Since all native 
[FileInputStream|FileOutputStream|RandomAccessFile].close0() methods do 
the same thing with FileDescriptor, I moved the method to FileDescriptor 
and made it static while adding an int fd (UNIX) / long handle (Windows) 
parameter. Clean-up action invokes this method when all connected 
stream(s) and FileHandle objects are already gone. The clean-up thunk is 
registered in initFd (UNIX) / initHandle (Windows) method invoked from 
JNI code of FileInputStream, FileOutputStream or RandomAccessFile (see 
modified SET_FD macro in io_util_md.h). Clean-up registration therefore 
only happens for FileInputStream, FileOutputStream and RandomAccessFile 
(the later therefore gets automatic cleanup which was not present 
before). All other usages of FileDescriptor throughout JDK don't 
register cleanup and therefore behave unchanged.

all java/io jtreg tests pass except of course the following two:

JT Harness : Tests that failed
java/io/FileInputStream/FinalizeShdCallClose.java: Test to ensure that 
FIS.finalize() invokes the close() method as per the specification.
java/io/FileOutputStream/FinalizeShdCallClose.java: Test to ensure that 
FOS.finalize() invokes the close() method as per the specification.


They don't even compile, since the removal of overriding 
File[Input|Output]Stream.finalize() method which throws IOException 
exposes Object.finalize() which throws Throwable. The tests override 
File[Input|Output]Stream classes and finalize() methods, calling 
super.close(). This change is therefore not entirely source-compatible, 
but removing finalization from the File streams can not be performed 
without the removal of the finalize() methods, so if this is the 
direction to be taken, such source-incompatibility has to be taken into 
account. I don't belive there are many File stream subclasses that 
override finalize() and call super.finalize(). Wait a second, there are 
two in JDK itself (Socket[Input|Output]Stream), but they override 
finalize() to "disable" finalization - making it a no-op. Removing both 
overrides therefore removes no-op finalization from Socket streams too...

The presented patch uses sun.misc.Cleaner for demonstration purposes 
only. I don't know if it's use in this scenario is correct since closing 
the file can be a blocking operation and Cleaner(s) are executed by 
ReferenceHandler thread. I think a PhantomCleaner presented in my 
proposal for JEP-132 which is executed by same thread(s) as finalizers 
is more suitable for this purpose.

So, what do you think? Is this the direction to evolve File stream 
automatic clean-up to?

Regards, Peter