RFR: 8060132: Handlers configured on abstract nodes in logging.properties are not always properly closed

[Mandy Chung]

On 11/4/14 8:48 AM, Daniel Fuchs wrote:
> Hi,
>
> Here is a revised patch that introduces a new
> <logger>.handlers.ensureCloseOnReset=true|false
> property.
>
You have explored several different options and as you have found,
none of them is ideal.  I think having a property to specify
ensureCloseOnReset would be reasonable to address the memory leak
concern I have.

Would it be better to name it <logger>.handlers.closeOnReset?

I am less sure if we need to have a global switch to revert
the default - do we expect the number of <logger>.handlers
is large? I would expect there would be small number.

> This property can be used as a fallback to turn the new behavior
> off in the improbable case where an application may need to do
> so. I have updated the LogManager class-level API documentation.
>
> http://cr.openjdk.java.net/~dfuchs/webrev_8060132/webrev.02/
>
line 104-114: s/"true"/{@code true}
{@code reset} should be {@link ...} to the reset method (the
first occurrance)

It doesn't seem necessary to specify "strongly referenced".
Maybe it's good to say "it will ensure the handlers associated
with the loggers will be closed at shutdown or at reset.
If "closeOnReset is false", application should ensure the
handlers be closed before the logger gets garbage collected."
- something along that line.

typos:
186:  explicitely -> explicitly

Perhaps PersistentLogger can be renamed to CloseOnResetLogger
to make it clear what these loggers are kept for.  Similarly
for the variable name "persistentLogger" may be renamed too.

917: final might be adding noise
940: persitentLoggers

line 917: you don't need (names.length == 0) to determine the default,
right?  if names.length == 0, it wouldn't add to the persistentLoggers list
as no iteration in the for loop.


Mandy