[ 
https://issues.apache.org/jira/browse/LOG4J2-149?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13552919#comment-13552919
 ] 

Ralph Goers commented on LOG4J2-149:
------------------------------------

It took me by surprise too when I first saw how the SMTPAppender worked, but I 
understand what it is trying to do. There isn't any danger of the isFilter 
method being called twice - that is only going to happen in the 
AppenderControl.  Note that this will NOT capture events that have been 
filtered by a global Filter, Logger Filter or Appender-ref Filter as they will 
never make it this far.  You would have to create a special global filter to 
capture the events if you really want to capture everything.

As for the patch, I'm not sure what it is accomplishing.  The CyclicBuffer add 
and removeAll methods are synchronized so wrapping those calls in a 
synchronized block doesn't do anything useful.  Removing buffer as a parameter 
from two methods and then having writeBuffer just reference the member variable 
also should accomplish nothing.  So I'm not ready to apply this patch until I 
understand how it fixes the problem.

It would also be helpful to have a test that demonstrates the problem.

                
> SMTPManager buffer access not synchronized; can result in empty emails
> ----------------------------------------------------------------------
>
>                 Key: LOG4J2-149
>                 URL: https://issues.apache.org/jira/browse/LOG4J2-149
>             Project: Log4j 2
>          Issue Type: Bug
>          Components: Appenders
>    Affects Versions: 2.0-beta4
>         Environment: N/A
>            Reporter: Scott Severtson
>         Attachments: SMTPManager-buffer-synchronization.patch
>
>
> If multiple error events are logged against the same SMTPAppender/Manager at 
> the same time, one email will contain both error messages, while the second 
> will be empty (no events).
> The original SMTPAppender patch included synchronization against the 
> CyclicBuffer to prevent such simultaneous access from multiple threads. This 
> appears to have been lost in the merge/refactor. Patch (to follow shortly) 
> re-introduces synchronization against the CyclicBuffer in the narrowest 
> possible scope.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to