On 7/4/14 6:35 PM, Alan Bateman wrote:
On 01/07/2014 10:25, Daniel Fuchs wrote:
Hi Jason, Alan,
Here is an updated version of the fix that does a bounded
retry (retries once - and if it fails, proceeds with the next
name). I have also added NO_FOLLOW_LINKS - for the case where
we try to open an existing Lockfile, and suppressed the
Files.isWritable check since that will be taken care of by
the call to FileChannel.open.
http://cr.openjdk.java.net/~dfuchs/webrev_8048020/webrev.01/
I also updated the comments to make it clear that the
file could not have been locked by another instance
of FileHandler (since that would have been taken care
of by our global 'locks' synchronization).
best regards,
-- daniel
This looks okay to me except for the zombie file case. I think I
missed the reason for using APPEND.
Given that nothing is going to be written to the file then maybe I don't
need APPEND.
I just don't want the call to FileChannel.open() to truncate the file.
Also you catch FileNotFoundException (which is not thrown by
FileChannel.open)
oh? What will FileChannel.open throw if the file does not exist then? Is
there another
exception? Or do you mean it's not possible to know why FileChannel.open
fails?
That would be bad...
so I wonder if you mean to catch IOException so that you handle all
possible I/O exceptions. If you meant IOException to handle any error
then the isWritable on the parent directory isn't needed (we can make
the isRegularFile check go away too if you want).
No I just want to catch the case where the file does not exist.
In the test case then the the use of getAbsolutePath seems redundant.
Also just to mention that setup method could use
Files.createTempDirectory.
The test has several @run lines and depends on the lines to be invoked
in sequence.
In particular this sequence:
32 * @run main/othervm CheckZombieLockTest CLEANUP
33 * @run main/othervm CheckZombieLockTest WRITABLE
34 * @run main/othervm CheckZombieLockTest CREATE_FIRST
35 * @run main/othervm CheckZombieLockTest CREATE_NEXT
36 * @run main/othervm CheckZombieLockTest CREATE_NEXT
37 * @run main/othervm CheckZombieLockTest CLEANUP
where it is important that the directory *is not* deleted between two
invocations.
The CLEANUP action will delete the directory if everything goes well,
and the try { } catch { } in main will take care of it if the test
fails...
-- daniel
-Alan
