[ 
https://issues.apache.org/jira/browse/LOG4J2-938?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14332159#comment-14332159
 ] 

Remko Popma commented on LOG4J2-938:
------------------------------------

I've given it some more thought and I would really prefer to avoid the 
complexity of having multiple Executor threads and manually starting/stopping 
them. I believe we will never be able to perfectly cover all corner cases. (My 
experience with managing the async loggers thread lifecycle in web applications 
has been fairly painful. For example, LOG4J2-493 is still open.)

We may be able to get the best of both worlds: for web apps, let's sidestep 
this issue entirely and use a null Executor. JMX notifications are done in the 
calling thread, and there is no more memory leak. Web apps are not low-latency 
so the performance cost should not be noticable. For non-web apps, we keep the 
current behaviour of a single static (daemon) Executor. In detail:

We introduce a new system property {{log4j2.jmx.notify.async}}. If {{true}} the 
Executor is initialized to a daemon background thread Executor. If {{false}}, 
the Executor is initialized to {{null}}.

If this property is not set (the most common case), we check if we are running 
in a web container:
* If we detect that the {{javax.servlet.Servlet}} class is on the classpath, we 
assume we are running in a web container and initialize the Executor to 
{{null}}.
* Otherwise we create a daemon Executor.

Both web apps and non-web apps can explicitly set the property to override the 
default behaviour if necessary.
Thoughts?

> org.apache.logging.log4j.core.jmx.Server never shuts down the ExecutorService 
> it creates
> ----------------------------------------------------------------------------------------
>
>                 Key: LOG4J2-938
>                 URL: https://issues.apache.org/jira/browse/LOG4J2-938
>             Project: Log4j 2
>          Issue Type: Bug
>          Components: JMX
>    Affects Versions: 2.1
>            Reporter: Mauro Molinari
>            Priority: Critical
>         Attachments: LOG4J2-938.patch
>
>
> The class {{org.apache.logging.log4j.core.jmx.Server}} creates an 
> {{ExecutorService}} at construction time and and stores it as an instance 
> variable of type {{Executor}} (named {{executor}}).
> This executor service is never shut down (I guess the {{unregisterMBeans()}} 
> methods may be good candidates, with some care for 
> {{unregisterMBeans(MBeanServer)}} which performs unregistration only for a 
> single {{MBeanServer}}). This causes a memory leak if Log4j is used in a web 
> application (under Tomcat, for instance) and the JMX services have been used 
> (i.e.: the {{Server}} class has been instantiated).
> But even worse, what I'm observing is that a notification Job may be 
> submitted to that executor by 
> {{javax.management.NotificationBroadcasterSupport.sendNotification(Notification)}},
>  invoked by 
> {{org.apache.logging.log4j.core.jmx.StatusLoggerAdmin.log(StatusData)}} in 
> certain circumstances exactly during Tomcat shutdown process: since the 
> executor is using non-daemon threads to execute tasks, this eventually 
> prevents the application server to shutdown (I have to kill it).



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to