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

[Jason Mehrens]

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

[Daniel Fuchs]

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

[Jason Mehrens]

Here is a test case.



public class GcOfLogger {

private static final String CLASS_NAME = GcOfLogger.class.getName();

public static void main(String[] args) throws Exception {
File log = new File(CLASS_NAME.concat(".xml"));
install(log.getCanonicalPath());
LogManager manager = LogManager.getLogManager();
boolean cleared = false;
for (int i = 0; i < 10; i++) {
if (manager.getLogger(CLASS_NAME) != null) {
System.runFinalization();
System.gc();
System.runFinalization();
Thread.sleep(200L);
} else {
cleared = true;
break;
}
}

if (!cleared) {
throw new IllegalStateException();
}

File lock = new File(log.getCanonicalPath().concat(".lck"));
System.out.println(log.delete());
System.out.println(lock.delete());
}

private static void install(String log) throws Exception {
String path = new File(log).getCanonicalPath();
Logger logger = Logger.getLogger(CLASS_NAME); //No strong reference.
logger.addHandler(new FileHandler(path));
}
}


----------------------------------------
> Date: Thu, 9 Oct 2014 22:59:43 +0200
> From: daniel.fuchs at oracle.com
> To: jason_mehrens at hotmail.com
> CC: core-libs-dev at openjdk.java.net
> Subject: Re: JDK-6774110 lock file is not deleted when child logger is used
>
> 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
>

[Stanimir Simeonoff]

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();
}
}
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).

Stanimir



On Thu, Oct 9, 2014 at 4:59 PM, Daniel Fuchs <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
>>
>
>

[Daniel Fuchs]

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

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

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

[Stanimir Simeonoff]

On Fri, Oct 10, 2014 at 5:13 PM, Jason Mehrens <jason_mehrens at hotmail.com>
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.
>
>
> 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.
>
>
> Actually we have framework that uses shared handlers quite liberally (like
writing lots of stuff in a common file) but it also keeps strong references
to any logger configured.
The framework doesn't use the properties file but a custom xml-based config.
My point is that: shared handlers may not be that uncommon and the change
may break existing code as any shared handler could be prematurely closed.

The Handler automatic closure remains problematic as they don't have a
defined lifecycle. close() should be invoked after there are no references
and it requires calling a potentially overridden method. It can be carried
by PhantomReference+WeakRefence combo, though.


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

[Daniel Fuchs]

On 10/10/14 16:36, Stanimir Simeonoff wrote:
> The Handler automatic closure remains problematic as they don't have a
> defined lifecycle. close() should be invoked after there are no
> references and it requires calling a potentially overridden method. It
> can be carried by PhantomReference+WeakRefence combo, though.

I agree. However we can't use sun.misc.Cleaner for that, and it's not
clear were we could/should poll() the reference queue in order to
close handler.

Somehow I feel that closing handlers *created by the configuration*
*in the configuration reset* might be the safest route.


best regards,

-- daniel

[Daniel Fuchs]

Hi,

I have logged https://bugs.openjdk.java.net/browse/JDK-8060132.
I will start a new thread for discussing this issue.

best regards,

-- daniel

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();
> }
> }
> 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).
>
> 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
>
>
>