JDK-6774110 lock file is not deleted when child logger is used

[Jason Mehrens]

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.fuchs at oracle.com
> To: stanimir at riflexo.com
> CC: jason_mehrens at hotmail.com; core-libs-dev at 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.fuchs at oracle.com
>> <mailto:daniel.fuchs at 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
>>
>>
>>
>