RFR: 6244047: impossible to specify directories to logging FileHandler unless they exist (original) (raw)
Jim Gish jim.gish at oracle.com
Tue Nov 13 21:30:18 UTC 2012
- Previous message: RFR: 6244047: impossible to specify directories to logging FileHandler unless they exist
- Next message: RFR: 6244047: impossible to specify directories to logging FileHandler unless they exist
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
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: jasonmehrens 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(), CREATENEW, WRITE); >> //fc = lockStream.getChannel(); >>>> CREATE rather than CREATENEW 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 CREATENEW. 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
- Previous message: RFR: 6244047: impossible to specify directories to logging FileHandler unless they exist
- Next message: RFR: 6244047: impossible to specify directories to logging FileHandler unless they exist
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]