[
https://issues.apache.org/jira/browse/LOG4J2-149?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13553280#comment-13553280
]
Scott Severtson commented on LOG4J2-149:
----------------------------------------
So, we came up with two possible approaches to solve this issue:
1. Make CyclicBuffer smarter, replacing .removeAll() with .removeTo(T item).
This would allow SMTPManager to only remove events *up to* the triggering
event, leaving anything appended after in the buffer. However, this has a
couple issues:
a. Empty or nonsensical emails (not containing the target event) could still
occur, if the CyclicBuffer is too small, and more events are written to the
buffer between the .isFiltered call and the .append call. This is actually an
issue with the current implementation as well.
b. Log4jLogEvent would need to (at a minimum) implement a .equals method, so
we can actually find the target event in the CyclicBuffer. I'm not sure if
.equals was omitted purposefully.
b. The .removeTo code would be a bit more complex than the current .removeAll
implementation.
2. Don't append non-filtered items in the .isFiltered method; instead, wait and
append them to the buffer just before the current .removeAll call. This would
ensure that at least the triggering event is reported. However, this also has
an issue: Currently, CyclicBuffer claims to be thread-safe, as all mutating
methods are synchronized. As .add would be called just before .removeAll, the
calling code in SMTPManager would need to instead hold a synchronization lock
(similar to my previous bad patch for this issue). Alternatively, .removeAll()
could become an atomic .removeAllAndInclude(T item), preserving the
thread-safety of CyclicBuffer, at the cost of some reusability of the class
(the method doesn't seem to make sense for general use cases).
I've done a proof of concept with #2, which only required changing a few lines
in a couple files. I did *not* yet make an atomic .removeAllAndInclude, but
could if this alternative is desired.
Thoughts?
> 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]
