While I agree with you on the principle that catching Throwable is rarely a 
good idea, I argue that this is one of the exceptions to the rule:

We want to unregister a LoggerContext, but if this fails we want to continue 
since failing to unregister a JMX MBean should not prevent Log4j from working. 
We want to capture and ignore any error here, including unanticipated ones. 

Sent from my iPhone

> On 29 Sep 2016, at 2:08, Gary Gregory <[email protected]> wrote:
> 
> -1: It's rarely a good idea to catch Throwable. I doubt you want to catch 
> OutOfMemoryError for example. Just say what you want to catch in a 
> mutli-catch. In this case, catch NoClassDefFoundError and Exception. 
> 
> In fact, since we need to deal with Android and Google App Engine 
> restrictions in a few places, we should document this someplace and try to 
> provide some common way to deal with this.
> 
> Gary
> 
> ---------- Forwarded message ----------
> From: <[email protected]>
> Date: Wed, Sep 28, 2016 at 9:23 AM
> Subject: logging-log4j2 git commit: LOG4J2-1506 patch by Johannes Schleger to 
> catch Throwable instead of Exception
> To: [email protected]
> 
> 
> Repository: logging-log4j2
> Updated Branches:
>   refs/heads/master 404d47502 -> 18c1f9f86
> 
> 
> LOG4J2-1506 patch by Johannes Schleger to catch Throwable instead of  
> Exception
> 
> 
> Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
> Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/18c1f9f8
> Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/18c1f9f8
> Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/18c1f9f8
> 
> Branch: refs/heads/master
> Commit: 18c1f9f8635349d84a7a57aaaaae41a1d3b72f92
> Parents: 404d475
> Author: rpopma <[email protected]>
> Authored: Thu Sep 29 01:23:39 2016 +0900
> Committer: rpopma <[email protected]>
> Committed: Thu Sep 29 01:23:39 2016 +0900
> 
> ----------------------------------------------------------------------
>  .../main/java/org/apache/logging/log4j/core/LoggerContext.java   | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> ----------------------------------------------------------------------
> 
> 
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/18c1f9f8/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java
> ----------------------------------------------------------------------
> diff --git 
> a/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java 
> b/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java
> index 1f99941..104a921 100644
> --- 
> a/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java
> +++ 
> b/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java
> @@ -315,8 +315,8 @@ public class LoggerContext extends AbstractLifeCycle
>              this.setStopping();
>              try {
>                  Server.unregisterLoggerContext(getName()); // LOG4J2-406, 
> LOG4J2-500
> -            } catch (final Exception ex) {
> -                LOGGER.error("Unable to unregister MBeans", ex);
> +            } catch (final Throwable t) {
> +                LOGGER.error("Unable to unregister MBeans", t);
>              }
>              if (shutdownCallback != null) {
>                  shutdownCallback.cancel();
> 
> 
> 
> 
> -- 
> E-Mail: [email protected] | [email protected] 
> Java Persistence with Hibernate, Second Edition
> JUnit in Action, Second Edition
> Spring Batch in Action
> Blog: http://garygregory.wordpress.com 
> Home: http://garygregory.com/
> Tweet! http://twitter.com/GaryGregory

Reply via email to