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

[Jim Gish]

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