You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by rp...@apache.org on 2016/09/28 16:23:49 UTC
logging-log4j2 git commit: LOG4J2-1506 patch by Johannes Schleger to
catch Throwable instead of Exception
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 <rp...@apache.org>
Authored: Thu Sep 29 01:23:39 2016 +0900
Committer: rpopma <rp...@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();
Re: logging-log4j2 git commit: LOG4J2-1506 patch by Johannes Schleger
to catch Throwable instead of Exception
Posted by Gary Gregory <ga...@gmail.com>.
On Thu, Sep 29, 2016 at 8:50 AM, Remko Popma <re...@gmail.com> wrote:
> Thank you.
>
YW :-)
>
> Sent from my iPhone
>
> On 29 Sep 2016, at 9:07, Gary Gregory <ga...@gmail.com> wrote:
>
> On Wed, Sep 28, 2016 at 3:47 PM, Gary Gregory <ga...@gmail.com>
> wrote:
>
>> On Wed, Sep 28, 2016 at 3:43 PM, Remko Popma <re...@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 <ga...@gmail.com> wrote:
>>>
>>> On Wed, Sep 28, 2016 at 10:18 AM, Remko Popma <re...@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 <ga...@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: <rp...@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/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 <rp...@apache.org>
>>>> Authored: Thu Sep 29 01:23:39 2016 +0900
>>>> Committer: rpopma <rp...@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: garydgregory@gmail.com | ggregory@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: garydgregory@gmail.com | ggregory@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: garydgregory@gmail.com | ggregory@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: garydgregory@gmail.com | ggregory@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: garydgregory@gmail.com | ggregory@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
Re: logging-log4j2 git commit: LOG4J2-1506 patch by Johannes Schleger to catch Throwable instead of Exception
Posted by Remko Popma <re...@gmail.com>.
Thank you.
Sent from my iPhone
> On 29 Sep 2016, at 9:07, Gary Gregory <ga...@gmail.com> wrote:
>
>> On Wed, Sep 28, 2016 at 3:47 PM, Gary Gregory <ga...@gmail.com> wrote:
>>> On Wed, Sep 28, 2016 at 3:43 PM, Remko Popma <re...@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 <ga...@gmail.com> wrote:
>>>>
>>>>> On Wed, Sep 28, 2016 at 10:18 AM, Remko Popma <re...@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 <ga...@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: <rp...@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 <rp...@apache.org>
>>>>>> Authored: Thu Sep 29 01:23:39 2016 +0900
>>>>>> Committer: rpopma <rp...@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
Re: logging-log4j2 git commit: LOG4J2-1506 patch by Johannes Schleger
to catch Throwable instead of Exception
Posted by Gary Gregory <ga...@gmail.com>.
On Wed, Sep 28, 2016 at 3:47 PM, Gary Gregory <ga...@gmail.com>
wrote:
> On Wed, Sep 28, 2016 at 3:43 PM, Remko Popma <re...@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 <ga...@gmail.com> wrote:
>>
>> On Wed, Sep 28, 2016 at 10:18 AM, Remko Popma <re...@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 <ga...@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: <rp...@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/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 <rp...@apache.org>
>>> Authored: Thu Sep 29 01:23:39 2016 +0900
>>> Committer: rpopma <rp...@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: garydgregory@gmail.com | ggregory@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: garydgregory@gmail.com | ggregory@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: garydgregory@gmail.com | ggregory@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: garydgregory@gmail.com | ggregory@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
Re: logging-log4j2 git commit: LOG4J2-1506 patch by Johannes Schleger
to catch Throwable instead of Exception
Posted by Gary Gregory <ga...@gmail.com>.
On Wed, Sep 28, 2016 at 3:43 PM, Remko Popma <re...@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...
Gary
>
> Sent from my iPhone
>
> On 29 Sep 2016, at 2:23, Gary Gregory <ga...@gmail.com> wrote:
>
> On Wed, Sep 28, 2016 at 10:18 AM, Remko Popma <re...@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 <ga...@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: <rp...@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 <rp...@apache.org>
>> Authored: Thu Sep 29 01:23:39 2016 +0900
>> Committer: rpopma <rp...@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: garydgregory@gmail.com | ggregory@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: garydgregory@gmail.com | ggregory@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: garydgregory@gmail.com | ggregory@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
Re: logging-log4j2 git commit: LOG4J2-1506 patch by Johannes Schleger to catch Throwable instead of Exception
Posted by Remko Popma <re...@gmail.com>.
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.
Sent from my iPhone
> On 29 Sep 2016, at 2:23, Gary Gregory <ga...@gmail.com> wrote:
>
>> On Wed, Sep 28, 2016 at 10:18 AM, Remko Popma <re...@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 <ga...@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: <rp...@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 <rp...@apache.org>
>>> Authored: Thu Sep 29 01:23:39 2016 +0900
>>> Committer: rpopma <rp...@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
Re: logging-log4j2 git commit: LOG4J2-1506 patch by Johannes Schleger
to catch Throwable instead of Exception
Posted by Mikael Ståldal <mi...@magine.com>.
I agree that we should narrow the catching to Exception and LinkageError.
On Thu, Sep 29, 2016 at 12:09 AM, Gary Gregory <ga...@gmail.com>
wrote:
> On Wed, Sep 28, 2016 at 10:23 AM, Gary Gregory <ga...@gmail.com>
> wrote:
>
>> On Wed, Sep 28, 2016 at 10:18 AM, Remko Popma <re...@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 <ga...@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: <rp...@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/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 <rp...@apache.org>
>>> Authored: Thu Sep 29 01:23:39 2016 +0900
>>> Committer: rpopma <rp...@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: garydgregory@gmail.com | ggregory@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: garydgregory@gmail.com | ggregory@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: garydgregory@gmail.com | ggregory@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
>
--
[image: MagineTV]
*Mikael Ståldal*
Senior software developer
*Magine TV*
mikael.staldal@magine.com
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.
Re: logging-log4j2 git commit: LOG4J2-1506 patch by Johannes Schleger
to catch Throwable instead of Exception
Posted by Matt Sicker <bo...@gmail.com>.
Narrowing it to LinkageError makes sense, go for it!
On 28 September 2016 at 17:09, Gary Gregory <ga...@gmail.com> wrote:
> On Wed, Sep 28, 2016 at 10:23 AM, Gary Gregory <ga...@gmail.com>
> wrote:
>
>> On Wed, Sep 28, 2016 at 10:18 AM, Remko Popma <re...@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 <ga...@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: <rp...@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/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 <rp...@apache.org>
>>> Authored: Thu Sep 29 01:23:39 2016 +0900
>>> Committer: rpopma <rp...@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: garydgregory@gmail.com | ggregory@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: garydgregory@gmail.com | ggregory@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: garydgregory@gmail.com | ggregory@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 <bo...@gmail.com>
Re: logging-log4j2 git commit: LOG4J2-1506 patch by Johannes Schleger
to catch Throwable instead of Exception
Posted by Gary Gregory <ga...@gmail.com>.
On Wed, Sep 28, 2016 at 10:23 AM, Gary Gregory <ga...@gmail.com>
wrote:
> On Wed, Sep 28, 2016 at 10:18 AM, Remko Popma <re...@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 <ga...@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: <rp...@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 <rp...@apache.org>
>> Authored: Thu Sep 29 01:23:39 2016 +0900
>> Committer: rpopma <rp...@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: garydgregory@gmail.com | ggregory@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: garydgregory@gmail.com | ggregory@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: garydgregory@gmail.com | ggregory@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
Re: logging-log4j2 git commit: LOG4J2-1506 patch by Johannes Schleger
to catch Throwable instead of Exception
Posted by Gary Gregory <ga...@gmail.com>.
On Wed, Sep 28, 2016 at 10:18 AM, Remko Popma <re...@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 <ga...@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: <rp...@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 <rp...@apache.org>
> Authored: Thu Sep 29 01:23:39 2016 +0900
> Committer: rpopma <rp...@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: garydgregory@gmail.com | ggregory@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: garydgregory@gmail.com | ggregory@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
Re: logging-log4j2 git commit: LOG4J2-1506 patch by Johannes Schleger to catch Throwable instead of Exception
Posted by Remko Popma <re...@gmail.com>.
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 <ga...@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: <rp...@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 <rp...@apache.org>
> Authored: Thu Sep 29 01:23:39 2016 +0900
> Committer: rpopma <rp...@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
Fwd: logging-log4j2 git commit: LOG4J2-1506 patch by Johannes
Schleger to catch Throwable instead of Exception
Posted by Gary Gregory <ga...@gmail.com>.
-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: <rp...@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 <rp...@apache.org>
Authored: Thu Sep 29 01:23:39 2016 +0900
Committer: rpopma <rp...@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
<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