RFR: 8048020 - Regression on java.util.logging.FileHandler

Daniel Fuchs daniel.fuchs at oracle.com
Fri Jul 4 17:25:14 UTC 2014

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 
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

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

-- daniel

> -Alan

More information about the core-libs-dev mailing list