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.g...@oracle.com
To: alan.bate...@oracle.com
CC: jason_mehr...@hotmail.com; core-libs-dev@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.g...@oracle.com <mailto:jim.g...@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.g...@oracle.com
