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

[Daniel Fuchs]

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
>