Thank you. Sent from my iPhone
> On 29 Sep 2016, at 9:07, Gary Gregory <garydgreg...@gmail.com> wrote: > >> On Wed, Sep 28, 2016 at 3:47 PM, Gary Gregory <garydgreg...@gmail.com> wrote: >>> On Wed, Sep 28, 2016 at 3:43 PM, Remko Popma <remko.po...@gmail.com> wrote: >>> Hm, ok. >>> OutOfMemoryError may actually be recoverable if GC occurs, but ThreadDeath >>> is probably not something we want to catch without rethrowing. Agree it's >>> better to narrow it down to Exception|LinkageError. >> >> Will do... > > Done. > > Gary > >> >> Gary >> >>> >>> Sent from my iPhone >>> >>>> On 29 Sep 2016, at 2:23, Gary Gregory <garydgreg...@gmail.com> wrote: >>>> >>>>> On Wed, Sep 28, 2016 at 10:18 AM, Remko Popma <remko.po...@gmail.com> >>>>> wrote: >>>>> 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. >>>> >>>> Why not narrow it down and catch LinkageError? I do not see how catching >>>> OutOfMemoryError or any VirtualMachineError is a good idea here. >>>> >>>> Gary >>>> >>>>> >>>>> Sent from my iPhone >>>>> >>>>>> On 29 Sep 2016, at 2:08, Gary Gregory <garydgreg...@gmail.com> 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: <rpo...@apache.org> >>>>>> 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: comm...@logging.apache.org >>>>>> >>>>>> >>>>>> 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 <rpo...@apache.org> >>>>>> Authored: Thu Sep 29 01:23:39 2016 +0900 >>>>>> Committer: rpopma <rpo...@apache.org> >>>>>> 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: garydgreg...@gmail.com | ggreg...@apache.org >>>>>> 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 >>>> >>>> >>>> >>>> -- >>>> E-Mail: garydgreg...@gmail.com | ggreg...@apache.org >>>> 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 >> >> >> >> >> -- >> E-Mail: garydgreg...@gmail.com | ggreg...@apache.org >> 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 > > > > -- > E-Mail: garydgreg...@gmail.com | ggreg...@apache.org > 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
