logging-log4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ralph Goers (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (LOG4J2-702) LoggerConfig#waitForCompletion is not thread safe
Date Sat, 12 Jul 2014 21:25:05 GMT

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

Ralph Goers commented on LOG4J2-702:
------------------------------------

Here is what I am thinking.
1. I really don't know why logEvent is part of PrivateConfig. It should be moved to the Logger
class itself.
2. Revert the changes Matt made.
3 Change LoggerConfig.log(LogEvent event) return a boolean and to check the Configuration
lifecycle state for "stopping" after incrementing the counter. Since we know stop() is only
called after updateLoggers is called we can just return false if that is the case.
3. Change LoggerConfig.log(params) to return the LogEvent if the log method returns false,
otherwise return null.
4. Change Logger.logMessage to do:
{code}
LogEvent event = config.loggerConfig.log(getName(), fqcn, marker, level, msg, t);
if (event != null) {
  if (!config.loggerConfig.log(event))
  {
     // do we loop here or throw an exception? Reconfiguration shouldn't happen 2 times in
a row this fast.
  }
}
{code}
5. Change Logger.logEvent to do:{code}
  if (!config.loggerConfig.log(event))
  {
     if (!config.loggerConfig.log(event))
     {    
        // do we loop here or throw an exception? Reconfiguration shouldn't happen 2 times
in a row this fast.
     }    
  }
{code}

The only other way I can think of to do this is the use a ReadLock when accessing the PrivateConfig
to log and a WriteLock in updateConfiguration, but I really don't want to do that.

> 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-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org


Mime
View raw message