Seán Coffey wrote:
http://bugs.sun.com/view_bug.do?bug_id=7082769

webrev : http://cr.openjdk.java.net/~coffeys/webrev.7082769.7087019.jdk8/

Bug fix where we ensure that the fd object is not disposed of until all streams are closed out.

Testcase is a bulked up version of CR 6322678 (which wasn't committed at time of 6322678 fix). It includes create/close() calls for FileInputStream/FileOutputStream/RandomAccessFile which all reference the same file descriptor. Multi threaded access to the same file descriptor is also tested.

Typo fix also as per http://bugs.sun.com/view_bug.do?bug_id=7087019 also included.

I think the change are okay. There are other issues with sharing file descriptors between streams but your changes are a good improvement and eliminate the messy runningFinalizer code (which I think someone added to address a regression from a previous fix to an address another issue with sharing file descriptors).

The coverage in the new test looks good but I think the code could be a bit cleaner. For example in TestFinalizer then it uses try-with-resources at L64 but not at L55. It uses /**/ style comments at L63 but // style at L76. Temporary file usage is also inconsistent, Test_ is created in the system temporary directory, Test6322678 in the working directory. Also the temporary file name is duplicated at L97. I think TestMultipleFD would be a lot cleaner with try-with-resources too. I also suspect the deletes at L138 and L156 may be failing on Windows. If you have time to do a bit of clean-up in the test then I think you are good to go.

-Alan.


Reply via email to