Henry Tung created LOG4J2-2954:
----------------------------------
Summary: Shutdown callbacks registered in
LoggerContext.setUpShutdownHook() aren't strongly referenced
Key: LOG4J2-2954
URL: https://issues.apache.org/jira/browse/LOG4J2-2954
Project: Log4j 2
Issue Type: Bug
Reporter: Henry Tung
LoggerContext.setUpShutdownHook() passes a callback to the
DefaultShutdownCallbackRegistry. To "avoid a memory leak", the registry only
holds a soft reference to the passed callback.
However, the callback is itself just an inline lambda not-otherwise-referenced.
Even while the context is alive, the callback will soon become eligible for GC
once the SoftReference timer-from-last-access expires. By the time the shutdown
hooks actually run, the references are cleared and the context is not
gracefully closed.
Probably surfaced after
[https://github.com/apache/logging-log4j2/commit/bee90521bb63e6162a9cbed606adde4242675c62.]
"Fixing" that leak (which actually kept the callback alive) now makes the
callback completely unreferenced (strongly).
An improvement would be to make RegisteredCancellable.hook a strong reference,
and instead to make DefaultShutdownCallbackRegistry.hooks a
Collection<WeakReference<...>>. This would keep the callback alive as long as
the returned registration object (the Cancellable) is alive, and the
LoggerContext does reference that.
------
Note: Unclear if the above is actually a complete solution. "LoggerContext
eligible for GC" doesn't seem to imply "LoggerContext contents all flushed to
disk" - the strong reference from the shutdown hook to the context actually
seems correct in that context, to guarantee that contents will be flushed on
finish.
Instead, re: "memory leak", it seems like there should be a burden on Log4j API
consumers to close old contexts they don't need anymore.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
