Discussion:
[jira] [Created] (LOG4J2-702) LoggerContext#waitForCompletion is not thread safe
Sean Bridges (JIRA)
2014-07-07 04:31:34 UTC
Permalink
Sean Bridges created LOG4J2-702:
-----------------------------------

Summary: LoggerContext#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


This is in trunk, svn commit 1608156

LoggerContext#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 LoggerContext object.



--
This message was sent by Atlassian JIRA
(v6.2#6252)
Matt Sicker (JIRA)
2014-07-07 04:57:34 UTC
Permalink
[ https://issues.apache.org/jira/browse/LOG4J2-702?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Matt Sicker updated LOG4J2-702:
-------------------------------

Priority: Critical (was: Major)
Post by Sean Bridges (JIRA)
LoggerContext#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
Priority: Critical
This is in trunk, svn commit 1608156
LoggerContext#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 LoggerContext object.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
Matt Sicker (JIRA)
2014-07-07 05:59:34 UTC
Permalink
[ https://issues.apache.org/jira/browse/LOG4J2-702?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14053370#comment-14053370 ]

Matt Sicker commented on LOG4J2-702:
------------------------------------

Oh man, good catch on a neat threading issue! Taking a look at what's going on here, I think a slightly more complex locking mechanism will be needed for the counter. Before this change, the whole method {{waitForCompletion}} was synchronized. It seems like this could be doable.
Post by Sean Bridges (JIRA)
LoggerContext#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
Priority: Critical
This is in trunk, svn commit 1608156
LoggerContext#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 LoggerContext object.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
Matt Sicker (JIRA)
2014-07-07 05:59:34 UTC
Permalink
[ https://issues.apache.org/jira/browse/LOG4J2-702?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Matt Sicker reassigned LOG4J2-702:
----------------------------------

Assignee: Matt Sicker
Post by Sean Bridges (JIRA)
LoggerContext#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
This is in trunk, svn commit 1608156
LoggerContext#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 LoggerContext object.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
Matt Sicker (JIRA)
2014-07-07 06:32:34 UTC
Permalink
[ https://issues.apache.org/jira/browse/LOG4J2-702?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Matt Sicker resolved LOG4J2-702.
--------------------------------

Resolution: Fixed
Fix Version/s: 2.0

Fixed in revision 1608344. Please verify and close.
Post by Sean Bridges (JIRA)
LoggerContext#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
LoggerContext#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 LoggerContext object.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
Ralph Goers (JIRA)
2014-07-07 06:34:33 UTC
Permalink
[ https://issues.apache.org/jira/browse/LOG4J2-702?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14053379#comment-14053379 ]

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

First, I have to assume the problem description is incorrect and that all references to LoggerContext should be LoggerConfig?

Second, the clearAppenders method is called by stop (and really shouldn't be called by anything else). The stop method should only be called after updateLoggers has been called to transfer logging to a new configuration, in which calls all logging events would go there, not to the current configuration or during a shutdown, at which point logging shouldn't be happening.

Do you have a stack trace that shows an actual problem or is this a hypothetical exercise?
Post by Sean Bridges (JIRA)
LoggerContext#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
LoggerContext#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 LoggerContext object.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
Matt Sicker (JIRA)
2014-07-07 06:40:33 UTC
Permalink
[ https://issues.apache.org/jira/browse/LOG4J2-702?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14053381#comment-14053381 ]

Matt Sicker commented on LOG4J2-702:
------------------------------------

I think it's a hypothetical actually. The clearAppenders method is protected and is only called by AbstractConfiguration.stop(). This follows a chain of stop() methods as well, so this might actually be a non-issue.
Post by Sean Bridges (JIRA)
LoggerContext#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
LoggerContext#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 LoggerContext object.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
Ralph Goers (JIRA)
2014-07-07 06:48:34 UTC
Permalink
[ https://issues.apache.org/jira/browse/LOG4J2-702?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14053383#comment-14053383 ]

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

Yes, and I have a fear that moving the lock as you did might cause a deadlock, although the ReconfigurationDeadlockTest which was added for LOG4J2-620 still passes.
Post by Sean Bridges (JIRA)
LoggerContext#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
LoggerContext#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 LoggerContext object.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
Ralph Goers (JIRA)
2014-07-07 06:48:34 UTC
Permalink
[ https://issues.apache.org/jira/browse/LOG4J2-702?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Ralph Goers updated LOG4J2-702:
-------------------------------

Description:
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.

was:
This is in trunk, svn commit 1608156

LoggerContext#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 LoggerContext object.

Summary: LoggerConfig#waitForCompletion is not thread safe (was: LoggerContext#waitForCompletion is not thread safe)

I changed all mentions of LoggerContext to LoggerConfig
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)
Ralph Goers (JIRA)
2014-07-07 06:50:33 UTC
Permalink
[ https://issues.apache.org/jira/browse/LOG4J2-702?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14053379#comment-14053379 ]

Ralph Goers edited comment on LOG4J2-702 at 7/7/14 6:50 AM:
------------------------------------------------------------

First, I have to assume the problem description is incorrect and that all references to LoggerContext should be LoggerConfig?

Second, the clearAppenders method is called by stop (and really shouldn't be called by anything else). The stop method should only be called after updateLoggers has been called to transfer logging to a new configuration, in which case calls to all log events would go there, not to the configuration being stopped, or during a shutdown, at which point logging shouldn't be happening.

Do you have a stack trace that shows an actual problem or is this a hypothetical exercise?


was (Author: ***@dslextreme.com):
First, I have to assume the problem description is incorrect and that all references to LoggerContext should be LoggerConfig?

Second, the clearAppenders method is called by stop (and really shouldn't be called by anything else). The stop method should only be called after updateLoggers has been called to transfer logging to a new configuration, in which calls all logging events would go there, not to the current configuration or during a shutdown, at which point logging shouldn't be happening.

Do you have a stack trace that shows an actual problem or is this a hypothetical exercise?
Post by Ralph Goers (JIRA)
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)
Ralph Goers (JIRA)
2014-07-07 06:52:34 UTC
Permalink
[ https://issues.apache.org/jira/browse/LOG4J2-702?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14053387#comment-14053387 ]

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

This would have been a good choice to ask a question on the dev list before immediately making a change.
Post by Ralph Goers (JIRA)
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)
Matt Sicker (JIRA)
2014-07-07 15:02:34 UTC
Permalink
[ https://issues.apache.org/jira/browse/LOG4J2-702?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14053727#comment-14053727 ]

Matt Sicker commented on LOG4J2-702:
------------------------------------

I'm going to revert this change and defer any additional changes until we decide a change is actually necessary. The idea behind this change was to make sure anything attempting to wait for completion has to wait to get a lock before it can even check the shutdown status. I don't think it's necessary based on current usage as long as we make a note in the javadocs to clarify as such.
Post by Ralph Goers (JIRA)
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)
Sean Bridges (JIRA)
2014-07-07 18:08:35 UTC
Permalink
[ https://issues.apache.org/jira/browse/LOG4J2-702?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14053946#comment-14053946 ]

Sean Bridges commented on LOG4J2-702:
-------------------------------------

{quote}
Second, the clearAppenders method is called by stop (and really shouldn't be called by anything else). The stop method should only be called after updateLoggers has been called to transfer logging to a new configuration, in which case calls to all log events would go there, not to the configuration being stopped, or during a shutdown, at which point logging shouldn't be happening.
{quote}

I think there is still the same problem just pushed up the stack a bit, as you can have two threads doing something like,

{code}
Thread B acquire reference to current loggerConfig

Thread A updateLoggers()
Thread A stop()
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}
Post by Ralph Goers (JIRA)
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)
Sean Bridges (JIRA)
2014-07-07 18:08:35 UTC
Permalink
[ https://issues.apache.org/jira/browse/LOG4J2-702?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Sean Bridges reopened LOG4J2-702:
---------------------------------
Post by Ralph Goers (JIRA)
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)
Ralph Goers (JIRA)
2014-07-07 18:18:34 UTC
Permalink
[ https://issues.apache.org/jira/browse/LOG4J2-702?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14053954#comment-14053954 ]

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

Do you have an actual stack trace that demonstrates the problem?
Post by Ralph Goers (JIRA)
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)
Ralph Goers (JIRA)
2014-07-07 18:20:34 UTC
Permalink
[ https://issues.apache.org/jira/browse/LOG4J2-702?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Ralph Goers updated LOG4J2-702:
-------------------------------

Comment: was deleted

(was: Do you have an actual stack trace that demonstrates the problem? )
Post by Ralph Goers (JIRA)
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)
Sean Bridges (JIRA)
2014-07-07 18:22:36 UTC
Permalink
[ https://issues.apache.org/jira/browse/LOG4J2-702?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14053964#comment-14053964 ]

Sean Bridges commented on LOG4J2-702:
-------------------------------------

{quote}
Do you have an actual stack trace that demonstrates the problem?
{quote}

I haven't experienced this problem. I was just curious if log4j2 was able to log without doing any locking like log4j does, and saw this code isn't thread safe.
Post by Ralph Goers (JIRA)
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)
Ralph Goers (JIRA)
2014-07-07 18:38:33 UTC
Permalink
[ https://issues.apache.org/jira/browse/LOG4J2-702?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14053989#comment-14053989 ]

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

I will look at this. I would expect the odds that it will ever happen are low but I guess it could happen, especially if you only have a root logger.
Post by Ralph Goers (JIRA)
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)
Sean Bridges (JIRA)
2014-07-07 18:50:34 UTC
Permalink
[ https://issues.apache.org/jira/browse/LOG4J2-702?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14054012#comment-14054012 ]

Sean Bridges edited comment on LOG4J2-702 at 7/7/14 6:50 PM:
-------------------------------------------------------------

I think the bookeeping needs to be done outside the LoggerConfig class.

Say you used loggerConfig something like,

{code}

class LoggerConfig {
/**
* returns true if the reference count was incremented
*/
public boolean addRef()...

public void decRef() ...
}

class SomeClassUsingLoggerConfig {
volatile LoggerConfig loggerConfig;


...
while(true) {
LoggerConfig local = loggerConfig;
if(local.addRef()) {
try {
//use local
break;
} finally {
local.decRef();
}
}
else {
//sleep? some sort of exponential backoff
}

}
}

{code}




was (Author: sbridges):
I think the bookeeping needs to be done outside the LoggerConfig class.

Say you used loggerConfig something like,

{code}

class LoggerConfig {
/**
* returns true if the reference count was incremented
*/
public boolean addRef()...

public void decRef() ...
}

class SomeClassUsingLoggerConfig {
volatile LoggerConfig loggerConfig;


...
while(true) {
LoggerConfig local = loggerConfig;
if(local.addRef()) {
try {
//use local
break;
} finally {
local.decRef();
}
else {
//sleep? some sort of exponential backoff
}
}
}
}

{code}
Post by Ralph Goers (JIRA)
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)
Sean Bridges (JIRA)
2014-07-07 18:50:34 UTC
Permalink
[ https://issues.apache.org/jira/browse/LOG4J2-702?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14054012#comment-14054012 ]

Sean Bridges commented on LOG4J2-702:
-------------------------------------

I think the bookeeping needs to be done outside the LoggerConfig class.

Say you used loggerConfig something like,

{code}

class LoggerConfig {
/**
* returns true if the reference count was incremented
*/
public boolean addRef()...

public void decRef() ...
}

class SomeClassUsingLoggerConfig {
volatile LoggerConfig loggerConfig;


...
while(true) {
LoggerConfig local = loggerConfig;
if(local.addRef()) {
try {
//use local
break;
} finally {
local.decRef();
}
else {
//sleep? some sort of exponential backoff
}
}
}
}

{code}
Post by Ralph Goers (JIRA)
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)
Remko Popma (JIRA)
2014-07-11 00:06:04 UTC
Permalink
[ 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?
Post by Ralph Goers (JIRA)
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)
Sean Bridges (JIRA)
2014-07-12 04:58:04 UTC
Permalink
[ https://issues.apache.org/jira/browse/LOG4J2-702?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14059646#comment-14059646 ]

Sean Bridges commented on LOG4J2-702:
-------------------------------------

I don't think your example code works, I added some comments below,

{code}
public void logEvent(final LogEvent event) {
loggerConfig.incRef();
try {
config.getConfigurationMonitor().checkConfiguration();
//loggerConfig here may not be == to the loggerConfig you called incRef() on
loggerConfig.log(event);
} finally {
//loggerConfig here may not be == to the loggerConfig you called incRef() on
loggerConfig.decRef();
}
}
{code}

This also doesn't work,

{code}
LoggerConfig origConfig = loggerConfig;
origConfig.incRef();
try {
//use origConfig
//however this is the same as the original problem
} finally {
origConfig.decRef();
}
{code}
Post by Ralph Goers (JIRA)
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)
Remko Popma (JIRA)
2014-07-12 05:19:04 UTC
Permalink
[ https://issues.apache.org/jira/browse/LOG4J2-702?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14059654#comment-14059654 ]

Remko Popma edited comment on LOG4J2-702 at 7/12/14 5:17 AM:
-------------------------------------------------------------

Sean, I believe your analysis is incorrect. The call to {{config.getConfigurationMonitor().checkConfiguration();}} may cause the Logger's PrivateConfig to be replaced by a fresh one, that is true. This fresh privateConfig willl contain a new LoggerConfig. The Logger will no longer have a reference to the old PrivateConfig and new {{logMessage()}} calls will be delegated to the new PrivateConfig object.

However, the code is currently executing in the old PrivateConfig and will now execute the next statement ({{loggerConfig.log(event);}}) on the old PrivateConfig object, which still has a reference to the old LoggerConfig.


was (Author: ***@yahoo.com):
Sean, I believe your analysis is incorrect. The call to {{config.getConfigurationMonitor().checkConfiguration();}} may cause the Logger's PrivateConfig to be replaced by a fresh one, that is true. This fresh privateConfig willl contain a new LoggerConfig. The Logger will no longer have a reference to the old PrivateConfig and new {{logMessage()}} calls will be delegated to the new PrivateConfig object.

However, the code is currently executing in the old PrivateConfig and will now execute the next statement ({{loggerConfig.log(event);}} on the old PrivateConfig object, which still has a reference to the old LoggerConfig.
Post by Ralph Goers (JIRA)
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)
Remko Popma (JIRA)
2014-07-12 05:19:04 UTC
Permalink
[ https://issues.apache.org/jira/browse/LOG4J2-702?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14059654#comment-14059654 ]

Remko Popma commented on LOG4J2-702:
------------------------------------

Sean, I believe your analysis is incorrect. The call to {{config.getConfigurationMonitor().checkConfiguration();}} may cause the Logger's PrivateConfig to be replaced by a fresh one, that is true. This fresh privateConfig willl contain a new LoggerConfig. The Logger will no longer have a reference to the old PrivateConfig and new {{logMessage()}} calls will be delegated to the new PrivateConfig object.

However, the code is currently executing in the old PrivateConfig and will now execute the next statement ({{loggerConfig.log(event);}} on the old PrivateConfig object, which still has a reference to the old LoggerConfig.
Post by Ralph Goers (JIRA)
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)
Remko Popma (JIRA)
2014-07-12 05:27:04 UTC
Permalink
[ https://issues.apache.org/jira/browse/LOG4J2-702?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14059654#comment-14059654 ]

Remko Popma edited comment on LOG4J2-702 at 7/12/14 5:26 AM:
-------------------------------------------------------------

Sean, I believe your analysis is incorrect. The call to {{config.getConfigurationMonitor().checkConfiguration();}} may cause the Logger's PrivateConfig to be replaced by a fresh one, that is true. This fresh privateConfig willl contain a new LoggerConfig. The Logger will no longer have a reference to the old PrivateConfig and going forward, invocations of the {{Logger.logMessage()}} method will be delegated to the new PrivateConfig object.

However, the code is currently executing in the old PrivateConfig instance and will now execute the next statement ({{loggerConfig.log(event);}}) on the old PrivateConfig object, which still has a reference to the old LoggerConfig.


was (Author: ***@yahoo.com):
Sean, I believe your analysis is incorrect. The call to {{config.getConfigurationMonitor().checkConfiguration();}} may cause the Logger's PrivateConfig to be replaced by a fresh one, that is true. This fresh privateConfig willl contain a new LoggerConfig. The Logger will no longer have a reference to the old PrivateConfig and new {{logMessage()}} calls will be delegated to the new PrivateConfig object.

However, the code is currently executing in the old PrivateConfig and will now execute the next statement ({{loggerConfig.log(event);}}) on the old PrivateConfig object, which still has a reference to the old LoggerConfig.
Post by Ralph Goers (JIRA)
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)
Ralph Goers (JIRA)
2014-07-12 19:11:05 UTC
Permalink
[ https://issues.apache.org/jira/browse/LOG4J2-702?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14059884#comment-14059884 ]

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

While I agree with you Remko, the problem really is that once you have a reference to the PrivateConfig you might still be incrementing the counter after waitForCompletion has determined the counter is at zero. What I am struggling with is how you can make
{code}
config.loggerConfig.log(getName(), fqcn, marker, level, msg, t);
{code}
in the logMessage method signal its intention to log just by accessing the config variable.
Post by Ralph Goers (JIRA)
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)
Ralph Goers (JIRA)
2014-07-12 21:25:05 UTC
Permalink
[ 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.
Post by Ralph Goers (JIRA)
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)
Remko Popma (JIRA)
2014-07-12 22:25:05 UTC
Permalink
[ https://issues.apache.org/jira/browse/LOG4J2-702?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14059942#comment-14059942 ]

Remko Popma commented on LOG4J2-702:
------------------------------------

PrivateLogger.logEvent() is called by AsyncLogger (from the background thread).
Post by Ralph Goers (JIRA)
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)
Remko Popma (JIRA)
2014-07-12 22:27:04 UTC
Permalink
[ https://issues.apache.org/jira/browse/LOG4J2-702?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14059942#comment-14059942 ]

Remko Popma edited comment on LOG4J2-702 at 7/12/14 10:25 PM:
--------------------------------------------------------------

I created the PrivateLogger.logEvent() method (a long time ago). It is called by AsyncLogger (from the background thread).


was (Author: ***@yahoo.com):
PrivateLogger.logEvent() is called by AsyncLogger (from the background thread).
Post by Ralph Goers (JIRA)
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)
Ralph Goers (JIRA)
2014-07-12 22:30:05 UTC
Permalink
[ https://issues.apache.org/jira/browse/LOG4J2-702?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14059945#comment-14059945 ]

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

Actually, I think I created it. It is also called by LogEventListener. However, I see no reason it shouldn't be part of the core Logger class instead of PrivateConfig.
Post by Ralph Goers (JIRA)
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)
Remko Popma (JIRA)
2014-07-12 23:55:05 UTC
Permalink
[ https://issues.apache.org/jira/browse/LOG4J2-702?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14059958#comment-14059958 ]

Remko Popma commented on LOG4J2-702:
------------------------------------

Ah, yes, I only increased its visibility, that's right. No objection to moving this method, btw.

I'm looking at the retry design. So far I haven't found any fault with it. I'll keep looking.

One thing: instead of retrying once (with an if), the standard idiom with lock-free designs like this seems to be a while loop that continues until it succeeds. We could have some max-attempts circuit breaker, but (as you pointed out) then what do we do with the event that we haven't been able to log? Log it to some error appender? Throwing an exception would disrupt the application which seems undesirable.
Post by Ralph Goers (JIRA)
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)
Ralph Goers (JIRA)
2014-07-13 00:03:04 UTC
Permalink
[ https://issues.apache.org/jira/browse/LOG4J2-702?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14059962#comment-14059962 ]

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

Yes, it would be undesirable. But the only time the retry should fail is if the PrivateConfig is attached to a Configuration that is once again marked as stopping. I would at least put a sleep for 10 milliseconds in the loop but I think looping for a fixed number of times and then throwing an exception would be fine as that would imply something bizarre is going on, as every iteration through the loop should be getting a new PrivateLogger since the one in use has a Configuration that is stopping.
Post by Ralph Goers (JIRA)
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)
Sean Bridges (JIRA)
2014-07-13 04:56:04 UTC
Permalink
[ https://issues.apache.org/jira/browse/LOG4J2-702?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14059991#comment-14059991 ]

Sean Bridges commented on LOG4J2-702:
-------------------------------------

Remko, I think your solution works.
Post by Ralph Goers (JIRA)
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)
Ralph Goers (JIRA)
2014-07-13 05:42:04 UTC
Permalink
[ https://issues.apache.org/jira/browse/LOG4J2-702?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14060004#comment-14060004 ]

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

Did you really mean Remko's solution or what I proposed? I don't think Remko's solution will work as the same scenario you outlined above can still occur after the code obtains the reference to the PrivateConfig but before logEvent increments the counter.
Post by Ralph Goers (JIRA)
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)
Sean Bridges (JIRA)
2014-07-14 03:55:04 UTC
Permalink
[ https://issues.apache.org/jira/browse/LOG4J2-702?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14060297#comment-14060297 ]

Sean Bridges commented on LOG4J2-702:
-------------------------------------

Sorry, I commented without seeing some of the posts.

Ralph, yes you are right, Remko's solution in his 10/Jul/14 17:05 comment suffers from the flaw you mention. I think it is better to loop rather than hardcode a fixed number of iterations. Another alternative to looping might be to chain the configs. Something like,

{code}
//in PrivateConfig
volatile LoggerConfig next;

void log(...) {
counter.incrementAndGet();
if(next != null) {
counter.decrementAndGet();
next.log(...);
} else {
try {
...do stuff
} finally {
counter.decrementAndGet();
}
}

void stop(LoggerConfig next) {
this.next = next;
...cleanup
}


}

{code}
Post by Ralph Goers (JIRA)
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)
Remko Popma (JIRA)
2014-07-18 12:12:05 UTC
Permalink
[ https://issues.apache.org/jira/browse/LOG4J2-702?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Remko Popma updated LOG4J2-702:
-------------------------------

Fix Version/s: (was: 2.0)
2.0.1
Post by Ralph Goers (JIRA)
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.1
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)
Remko Popma (JIRA)
2014-08-05 23:23:12 UTC
Permalink
[ https://issues.apache.org/jira/browse/LOG4J2-702?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Remko Popma updated LOG4J2-702:
-------------------------------

Fix Version/s: (was: 2.0.1)
2.1
Post by Ralph Goers (JIRA)
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.1
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)
Remko Popma (JIRA)
2014-09-24 09:43:34 UTC
Permalink
[ https://issues.apache.org/jira/browse/LOG4J2-702?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Remko Popma updated LOG4J2-702:
-------------------------------
Fix Version/s: (was: 2.1)
2.2
Assignee: (was: Matt Sicker)

The Fix Version for this issue was 2.1. Ralph (I'm assuming you want to take care of this issue), of course it would be great if you want to address this in the next few days, but I'm tentatively changing the Fix Version to 2.2 to give us some more time.
Post by Ralph Goers (JIRA)
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
Priority: Critical
Fix For: 2.2
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.3.4#6332)

Loading...