Re: Code review request: 7082769: FileInputStream/FileOutputStream/RandomAccessFile allow file descriptor be closed when still in use

2011-09-13 Thread Alan Bateman
Seán Coffey wrote: Cleaned up testcase (as per suggestions) is in latest webrev : http://cr.openjdk.java.net/~coffeys/webrev.7082769.7087019.jdk8.2/ Updated test looks fine to me. Minor comment is that I assume the /timeout=100 in the @run tag isn't needed. -Alan.

Re: Code review request: 7082769: FileInputStream/FileOutputStream/RandomAccessFile allow file descriptor be closed when still in use

2011-09-12 Thread Seán Coffey
Cleaned up testcase (as per suggestions) is in latest webrev : http://cr.openjdk.java.net/~coffeys/webrev.7082769.7087019.jdk8.2/ I've kept the try-with-resouces approach out of the TestMultipleFD method. It just complicates matters and with the closing order being important, it's easier to

Re: Code review request: 7082769: FileInputStream/FileOutputStream/RandomAccessFile allow file descriptor be closed when still in use

2011-09-09 Thread Chris Hegarty
Sean, The changes look good, though I haven't gone through the test in much detail! One question that is not directly related to your changes, but may be applicable. Shouldn't the close on FileChannel also check the value of the use count before closing the native fd? For example:

Re: Code review request: 7082769: FileInputStream/FileOutputStream/RandomAccessFile allow file descriptor be closed when still in use

2011-09-09 Thread Alan Bateman
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

Re: Code review request: 7082769: FileInputStream/FileOutputStream/RandomAccessFile allow file descriptor be closed when still in use

2011-09-09 Thread Seán Coffey
Good points Alan. Coding styles probably differ from merging two testcases together. I'll clean up on the issues mentioned and send out the new webrev shortly. I thought about try with resources in a few places but it didn't always suit. Take for example the TestMultipleFD() method. The