I could see rethrowing vm errors in some cases but we definitely don't want to
allow logging to stop entirely when these things occur.
On Wed, Oct 28, 2020, at 17:17, Gary Gregory 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.
> >
>
-ck
