[
https://issues.apache.org/jira/browse/LOG4J2-702?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14058141#comment-14058141
]
Remko Popma commented on LOG4J2-702:
------------------------------------
I think I agree with Sean's analysis. There is a small window of opportunity
between thread A obtaining a reference to the LoggerConfig and the counter
being incremented.
To flip this around, if the counter was incremented before the reference was
obtained, that would close the window.
When thinking about how to address this, I arrived at something close to Sean's
suggestion to have {{incRef()}} and {{decRef()}} methods on LoggerConfig.
How about
{code}
// Logger
public void logMessage(String fqcn, Level level, Marker marker, Message
message, final Throwable t) {
config.log(getName(), fqcn, marker, level, msg, t); // delegate all the
work to PrivateConfig
}
protected class PrivateConfig {
....
public void logEvent(final LogEvent event) {
loggerConfig.incRef();
try {
config.getConfigurationMonitor().checkConfiguration();
loggerConfig.log(event);
} finally {
loggerConfig.decRef();
}
}
public log(String name, String fqcn, Marker marker, Level level, Message
msg, Throwable t)
loggerConfig.incRef();
try {
config.getConfigurationMonitor().checkConfiguration();
final Message message = msg == null ? new
SimpleMessage(Strings.EMPTY) : msg;
loggerConfig.log(name, fqcn, marker, level, message, t);
} finally {
loggerConfig.decRef();
}
}
}
{code}
I'm not sure about Sean's suggestion to make the {{incRef()}} method return a
boolean and loop in the log() method until LoggerConfig.incRef() returns true.
With our current design, the LoggerConfig itself has been replaced and once it
has been shut down it will not become usable again. So once incRef() returned
false it will not return true after that.
However, isn't that problem already solved by the Logger.PrivateConfig being
replaced with a fresh instance (containing a different LoggerConfig) when a
reconfiguration occurs?
> LoggerConfig#waitForCompletion is not thread safe
> -------------------------------------------------
>
> Key: LOG4J2-702
> URL: https://issues.apache.org/jira/browse/LOG4J2-702
> Project: Log4j 2
> Issue Type: Bug
> Components: Core
> Affects Versions: 2.0-rc2
> Reporter: Sean Bridges
> Assignee: Matt Sicker
> Priority: Critical
> Fix For: 2.0
>
>
> This is in trunk, svn commit 1608156
> LoggerConfig#waitForCompletion uses an AtomicInteger counter to try to detect
> if there are any calls currently executing the log(Event) method, but it does
> not do so in a thread safe manner. Consider two threads A and B, where
> Thread A is calling clearAppenders(), and Thread B is calling log(Event),
> {code}
> Thread A loggerConfig.clearAppenders()
> Thread A loggerConfig.waitForCompletion()
> Thread A counter.get() //returns 0
> Thread A //loggerConfig.waitForCompletion() returns
> Thread B loggerConfig.log(Event)
> Thread B counter.increment()
> Thread A proceeds assuming no log calls are onging, but thread B is in the
> log method
> {code}
> I'm not sure what the requirements are, but if the requirement is to not lose
> logging events, I think you need some sort of synchronization outside of the
> LoggerConfig object.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscr...@logging.apache.org
For additional commands, e-mail: log4j-dev-h...@logging.apache.org
