Narrowing it to LinkageError makes sense, go for it!

On 28 September 2016 at 17:09, Gary Gregory <garydgreg...@gmail.com> wrote:

> On Wed, Sep 28, 2016 at 10:23 AM, 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.
>>
>
> If no one objects, I make this change...
>
> Gary
>
>
>>
>> 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/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 <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/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: garydgreg...@gmail.com | ggreg...@apache.org
>>> 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: garydgreg...@gmail.com | ggreg...@apache.org
>> 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: garydgreg...@gmail.com | ggreg...@apache.org
> 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
>



-- 
Matt Sicker <boa...@gmail.com>

Reply via email to