You might as well catch Throwable. You can’t really handle out of memory errors
- the JVM won’t let you. As for StackOverflowError - yes, that should be
handled. We don’t want the application to fail due to a buggy logging plugin.
Ralph
> On Oct 28, 2020, at 2:17 PM, Gary Gregory <garydgreg...@gmail.com> wrote:
>
> Hi, I am not sure it is wise to catch VM Errors like out of memory and
> stack overflow Errors... any thoughts on those?
>
> Gary
>
> On Wed, Oct 28, 2020, 16:05 Volkan Yazıcı <volkan.yaz...@gmail.com> wrote:
>
>> Hello,
>>
>> While logging, we sometimes notice that the entire logging infra comes to a
>> halt, even though the rest of the application still works perfectly fine. I
>> have figured, appenders wrapped by AsyncAppender can throw a
>> java.lang.Throwable that is not a subclass of java.lang.Exception and such
>> an exception kills the AsyncAppender worker thread. Consider the following
>> snippet from AsyncAppender.java in 2.x branch:
>>
>> boolean callAppenders(final LogEvent event) {
>> boolean success = false;
>> for (final AppenderControl control : appenders) {
>> try {
>> control.callAppender(event);
>> success = true;
>> } catch (final Exception ex) {
>> // If no appender is successful the error appender will get it.
>> }
>> }
>> return success;
>> }
>>
>> Further, this is the relevant AppenderControl#tryCallAppender(LogEvent)
>> method:
>>
>> private void tryCallAppender(final LogEvent event) {
>> try {
>> appender.append(event);
>> } catch (final RuntimeException ex) {
>> handleAppenderError(event, ex);
>> } catch (final Exception ex) {
>> handleAppenderError(event, new AppenderLoggingException(ex));
>> }
>> }
>>
>> To avoid AsyncAppender.AsyncThread getting killed, I propose, in
>> tryCallAppender(), replacing the java.lang.Exception catch clause with
>> java.lang.Throwable instead. Objections? (If there are none, I will push
>> this to both master and release-2.x branches with some unit tests.)
>>
>> To get some inspiration, I have checked the
>> java.util.concurrent.ThreadPoolExecutor#runWorker() method:
>>
>> try {
>> task.run();
>> } catch (RuntimeException x) {
>> thrown = x; throw x;
>> } catch (Error x) {
>> thrown = x; throw x;
>> } catch (Throwable x) {
>> thrown = x; throw new Error(x);
>> }
>>
>> This is inline with the change I propose for tryCallAppender().
>>
>> For the records, the most frequent Throwable we encounter that is a super
>> class of Exception is ExceptionInInitializerError, in case you are
>> interested in.
>>
>> Kind regards.
>>
