logging-log4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Remko Popma <remko.po...@gmail.com>
Subject Re: logging-log4j2 git commit: LOG4J2-1506 patch by Johannes Schleger to catch Throwable instead of Exception
Date Thu, 29 Sep 2016 15:50:22 GMT
Thank you. 

Sent from my iPhone

> On 29 Sep 2016, at 9:07, Gary Gregory <garydgregory@gmail.com> wrote:
> 
>> On Wed, Sep 28, 2016 at 3:47 PM, Gary Gregory <garydgregory@gmail.com> wrote:
>>> On Wed, Sep 28, 2016 at 3:43 PM, Remko Popma <remko.popma@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 <garydgregory@gmail.com> wrote:
>>>> 
>>>>> On Wed, Sep 28, 2016 at 10:18 AM, Remko Popma <remko.popma@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 <garydgregory@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: <rpopma@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: commits@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 <rpopma@apache.org>
>>>>>> Authored: Thu Sep 29 01:23:39 2016 +0900
>>>>>> Committer: rpopma <rpopma@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: garydgregory@gmail.com | ggregory@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: garydgregory@gmail.com | ggregory@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: garydgregory@gmail.com | ggregory@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: garydgregory@gmail.com | ggregory@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

Mime
View raw message