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

[Daniel Fuchs]

On 10/10/14 16:13, Jason Mehrens wrote:
> 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.

Hi Jason,

I discounted the idea because the same handler could be added
to different loggers (though not from the configuration).
Oh - that's what you're saying just below :-)

> 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.

I believe that keeping a reference on the loggers for which
a handler is created from the configuration file might be less
risky. After all - that's how it works for the root logger.

Hmmm... That's probably going to force me to evaluate and fix (at least
partly) https://bugs.openjdk.java.net/browse/JDK-8033661 as well...

best regards

-- daniel


>
>
> 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
>>>
>>>
>>>
>>