RFR: 6244047: impossible to specify directories to logging FileHandler unless they exist

[Jim Gish]

Here's a new webrev with my latest changes for your reviewing pleasure :-)

http://cr.openjdk.java.net/~jgish/Bug6244047-FileHandler-CheckLockLocation/ 
<http://cr.openjdk.java.net/%7Ejgish/Bug6244047-FileHandler-CheckLockLocation/>

Main changes:
- Using the file channel directly as suggested.
- Instead of checking up front for a valid directory I check the 
IOException on the channel open and process it accordingly. (BTW, I much 
prefer my previous proposed fix because I like to ensure pre-conditions 
for an operation are met prior to it rather than attempting the op, 
failing, and then recovering),
- Eliminated the stream, which really isn't needed, and just use the 
file channel

Just for the purposes of my enlightenment, assuming this is what you 
were after (Jason & Alan), what was your issue with checking for a valid 
directory up-front?

Thanks,
    Jim

On 11/13/2012 03:52 PM, Jim Gish wrote:
>
> On 11/13/2012 03:36 PM, Jason Mehrens wrote:
>> Jim,
>>
>> Looking at the webrev again I think I tricked myself into thinking 
>> that 'parentFile' was a file that could be opened with a stream when 
>> it actually is a directory.  I think the best fix would be to add a 
>> check in the catch block (around line 432) and only continue if the 
>> directory of the generated file 
>> java.nio.file.Files.isWritable/isDirectory otherwise throw the 
>> original exception.
> I have a variant, which I think may do the trick which I'll post 
> shortly. The catch block on 432 is for when the tryLock fails.  I 
> think it's best to check the IOException around 420.
>>
>> If the running JVM terminates abnormally won't the lock file fail to 
>> be deleted?  On restart, the lock file will exist to protect a dead 
>> process.
> Yes, except that you can't really distinguish between that and another 
> JVM using the lock file.  It's safer just to grab another file -- 
> which is what the logic is already setup to do.
>>
>> Jason
>>
>>
>> ------------------------------------------------------------------------
>> Date: Tue, 13 Nov 2012 13:25:27 -0500
>> From: jim.gish at oracle.com
>> To: Alan.Bateman at oracle.com
>> CC: jason_mehrens at hotmail.com; core-libs-dev at openjdk.java.net
>> Subject: Re: RFR: 6244047: impossible to specify directories to 
>> logging FileHandler unless they exist
>>
>>
>> On 11/13/2012 07:08 AM, Alan Bateman wrote:
>>
>>     On 12/11/2012 23:22, Jim Gish wrote:
>>
>>         Which file(s) are you concerned about truncating/damaging? 
>>         The code I'm impacting is for creating a new lock file.  Where
>>         is the potential for truncating/damaging that you both are
>>         referring to?
>>
>>         Is this sufficient (plus the proper exception handling of
>>         course) ?
>>
>>                             //lockStream = new
>>         FileOutputStream(lockFileName);
>>                             fc = FileChannel.open(new
>>         File(lockFileName).toPath(), CREATE_NEW, WRITE);
>>                             //fc = lockStream.getChannel();
>>
>>     CREATE rather than CREATE_NEW so that it doesn't fail if the lock
>>     file already exists. Although it's just a lock file then I think
>>     it would be impolite to truncate it.
>>
>>     You could use Paths.get(lockFileName)rather than new
>>     File(lockFileName).toPath() here but either is fine.
>>
>>     -Alan.
>>
>> I think we want it to fail if the lock file already exists. That's 
>> why I used CREATE_NEW.  At least the way the logic is now, is that it 
>> attempts to create a new file and if it fails it tries again until it 
>> can create a new one.  It may be the case that the lockFileName is 
>> not in the locks map, and thus we don't own it, but it's possible 
>> that another JVM/app is logging in the same location.  It, of course, 
>> would be bad practice, but not disallowed.  We shouldn't be grabbing 
>> a lock file that might otherwise be in use.
>>
>> Jim
>> -- 
>> Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304
>> Oracle Java Platform Group | Core Libraries Team
>> 35 Network Drive
>> Burlington, MA 01803
>> jim.gish at oracle.com  <mailto:jim.gish at oracle.com>
>

-- 
Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304
Oracle Java Platform Group | Core Libraries Team
35 Network Drive
Burlington, MA 01803
jim.gish at oracle.com