I agree that we should narrow the catching to Exception and LinkageError. On Thu, Sep 29, 2016 at 12:09 AM, Gary Gregory <[email protected]> wrote:
> On Wed, Sep 28, 2016 at 10:23 AM, Gary Gregory <[email protected]> > wrote: > >> On Wed, Sep 28, 2016 at 10:18 AM, Remko Popma <[email protected]> >> 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. >> > > If no one objects, I make this change... > > Gary > > >> >> Gary >> >> >>> 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/1 >>> 8c1f9f8 >>> Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/1 >>> 8c1f9f8 >>> >>> 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/1 >>> 8c1f9f8/log4j-core/src/main/java/org/apache/logging/log4j/co >>> re/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/Log >>> gerContext.java >>> index 1f99941..104a921 100644 >>> --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/Log >>> gerContext.java >>> +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/Log >>> gerContext.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 >>> <http://www.manning.com/bauer3/> >>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/> >>> Spring Batch in Action <http://www.manning.com/templier/> >>> Blog: http://garygregory.wordpress.com >>> Home: http://garygregory.com/ >>> Tweet! http://twitter.com/GaryGregory >>> >>> >> >> >> -- >> E-Mail: [email protected] | [email protected] >> Java Persistence with Hibernate, Second Edition >> <http://www.manning.com/bauer3/> >> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/> >> Spring Batch in Action <http://www.manning.com/templier/> >> Blog: http://garygregory.wordpress.com >> Home: http://garygregory.com/ >> Tweet! http://twitter.com/GaryGregory >> > > > > -- > E-Mail: [email protected] | [email protected] > Java Persistence with Hibernate, Second Edition > <http://www.manning.com/bauer3/> > JUnit in Action, Second Edition <http://www.manning.com/tahchiev/> > Spring Batch in Action <http://www.manning.com/templier/> > Blog: http://garygregory.wordpress.com > Home: http://garygregory.com/ > Tweet! http://twitter.com/GaryGregory > -- [image: MagineTV] *Mikael Ståldal* Senior software developer *Magine TV* [email protected] Grev Turegatan 3 | 114 46 Stockholm, Sweden | www.magine.com Privileged and/or Confidential Information may be contained in this message. If you are not the addressee indicated in this message (or responsible for delivery of the message to such a person), you may not copy or deliver this message to anyone. In such case, you should destroy this message and kindly notify the sender by reply email.
