[logback-dev] Question: AsyncAppender's thread safety?

[Aaron Curley]

Hello all,

I’m most likely missing something here, but reviewing AsyncAppender in logback classic 1.1.6, I’m not sure how it’s entirely thread safe, especially with respect to the “started” flag in its grandparent class (UnsynchronizedAppenderBase).

In particular, the doAppend() method in UnsynchronizedAppenderBase checks the “started” boolean member's value (line 72) before actually appending, yet, no synchronization primitives are (or appear to be, from my perspective) used to ensure that the “started” member is visible to all threads once it is updated via start().  I would have thought that AsyncAppender would have inherited from AppenderBase (which is thread safe and does mark “started” as volatile) rather than extending UnsynchronizedAppenderBase.  Baring this, I would have thought that the “started” member would be “overridden” with AsyncAppender’s own volatile version of this flag or access to started would be protected with a lock.

As a result, isn’t it possible that if one thread calls start() and then another thread immediately logs something (calling doAppend()), the logging thread would see “started” as false?  Wouldn’t this result in “skipping” certain log messages entirely (if the appender was very recently started)?

I think I must be missing something.  Can someone clarify how this works?  Thanks in advance!

Best Regards,

Aaron Curley
accwebs at gmail.com


P. S.  I would have also expected start() and stop() (in AsyncAppender or its parent) to be overridden to introduce synchronization, since AsyncAppender is generally used by multiple threads.  Right now, it appears that start & stop operations are not idempotent nor are multiple (simultaneous) calls prevented.