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
[email protected]


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.
_______________________________________________
logback-dev mailing list
[email protected]
http://mailman.qos.ch/mailman/listinfo/logback-dev

Reply via email to