Hi Daniel, Stanimir,
Closing the Handler is the main goal which takes care of the lock files. As far as a strong reference to the logger you don't need that. What you need to do is store a reference to the Logger.handlers List in the LogManager$LoggerWeakRef and reap the handlers inside of dispose. Now the only other issue is if one handler has been added to multiple loggers which could close a handler early. That is so uncommon I don't think it is worth the effort to correct. The caller can just fix the code to use a strong reference. Jason ---------------------------------------- > Date: Fri, 10 Oct 2014 11:39:41 +0200 > From: daniel.fu...@oracle.com > To: stani...@riflexo.com > CC: jason_mehr...@hotmail.com; core-libs-dev@openjdk.java.net > Subject: Re: JDK-6774110 lock file is not deleted when child logger is used > > Hi Stanimir, Jason, > > On 10/10/14 10:02, Stanimir Simeonoff wrote: >> Hi, >> >> LogManager.reset() should invoke a package private method to delete all >> lock files. However, that would require holding the FileHandler.locks >> monitor during the resetting of FileHandlers, not just the deletion >> process. Something like that, plus new PrivilegedAction(). >> static void deleteAllLocks(){ >> synchronized(locks){ >> for (String file : locks) new File(file).delete(); >> locks.clear(); >> } >> } > > There's more than the deletion of the lock file unfortunately. I believe > the handlers should be properly closed. A handler with an XMLFormatter > for instance needs to write the tail of the file. > > >> Alternatively the deletion could just be part of the Cleaner >> shutdownhook with another sun.misc.Cleaner per FileHandler that deletes >> the file. (Handlers can be shared amongst loggers, so they cannot be >> closed explicitly). There is a certain risk as file.delete() can be a >> very slow operation, though (ext3 [concurrently] deleting large files >> for example). > > That's a solution I envisaged and rejected because of the constraints > we have when running in the ReferenceHandler thread. I don't think it > would be appropriate to close a Handler in that thread. > > I'm leaning towards suggesting that the LogManager should hold a strong > reference on the loggers for which a Handler is explicitly > configured in the configuration file. It would ensure that > these loggers are still around when reset() is called. > > best regards, > > -- daniel > >> >> Stanimir >> >> >> >> On Thu, Oct 9, 2014 at 4:59 PM, Daniel Fuchs <daniel.fu...@oracle.com >> <mailto:daniel.fu...@oracle.com>> wrote: >> >> Thanks Jason. >> >> I wonder if that may be another issue. Interesting. I'll see if I >> can work out a test case >> for that tomorrow. With the test case provided in the bug - tested >> on 7, 8, and 9, >> the only file that remained at the end was 'log' (which is as it >> should be - and I ran >> the test case several times with each JDK) - which lets me think >> that maybe the >> issue was different. >> >> Now what you describe looks indeed like a bug that should still be >> present >> in the code base. I didn't think about that scenario, thanks for >> pointing it out! >> If i can write a reproducer (which should not be too difficult), it >> will be a good >> incentive to attempt a fix :-) >> >> Thanks again, >> >> -- daniel >> >> >> On 10/9/14 9:56 PM, Jason Mehrens wrote: >> >> Daniel, >> >> >> The evaluation on this bug is not quite correct. What is going >> on here is the child logger is garbage collected which makes the >> FileHandler unreachable from the LogManager$Cleaner which would >> have closed the attached FileHandler. In the example, there is >> no hard reference that escapes the 'execute' method. Prior to >> fixing JDK-6274920: JDK logger holds strong reference to >> java.util.logging.Logger instances, the LogManager$Cleaner would >> have deleted the lock file on shutdown. Now that the loggers >> are GC'able, one possible fix would be change the >> FileHandler.locks static field to Map<String,FileHandler> where >> the key is the file name and the value is the FileHandler that >> is open. Then in the LogManager$Cleaner could close any entries >> in that map after LogManager.reset() is executed. >> >> >> Jason >> >> >> >