activemq-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sam hendley (JIRA)" <j...@apache.org>
Subject [jira] [Created] (AMQ-4512) MemoryUsage left in inconsistent state
Date Mon, 06 May 2013 22:14:16 GMT
Sam hendley created AMQ-4512:
--------------------------------

             Summary: MemoryUsage left in inconsistent state
                 Key: AMQ-4512
                 URL: https://issues.apache.org/jira/browse/AMQ-4512
             Project: ActiveMQ
          Issue Type: Bug
          Components: Broker
    Affects Versions: 5.7.0, 5.6.0
            Reporter: Sam hendley


There is a race condition in MemoryUsage which makes it possible for it to be left in an inconsistent
state and thereby hang any clients in waitForSpace().

The core issue is in the following block of code:
{code:java}
    public void decreaseUsage(long value) {
        if (value == 0) {
            return;
        }
        int percentUsage;
        synchronized (usageMutex) {
            usage -= value;
            percentUsage = caclPercentUsage();
        }
        setPercentUsage(percentUsage);
        if (parent != null) {
            parent.decreaseUsage(value);
        }
    }
{code}

The bug occurs when multiple threads are calling increment/decrement at same time. Since the
field "usage" is protected with the usageMutex each writer/reader will see the correct and
current value for usage and calculate the right value for percentUsage at that instant. "setPercentUsage"
is also   protected by the same usageMutex so we resyncronize on usageMutex to set the percentUsage
field as well. The issue is that threads may enter the setPercentUsage synchronized block
in a different order than they entered the percentUsage "calculating" block. Since percentUsage
is carried between the two blocks, a reordering of threads can allow the wrong final percentUsage
value to be set. 

Possible threading (imagine usage starts at 0 and limit is 100).
Thread #1 - usage += 150; percentUsage = 150;
Thread #1 - suspended before called setPercentUsage
Thread #2 - usage -= 150; percentUsage = 0;
Thread #2 - setPercentUsage(0);
Thread #1 - resumed, can now call setPercentUsage
Thread #1 - setPercentUsage(150);

Final value = 150

This same pattern of synchronizing to calculate the percentUsage and then setting the value
later is repeated in all of the Usage objects I looked at. My guess it was written this way
to avoid holding locks while making calls out to "untrusted code" but it is a very dangerous
way to do the calculations. The most disappointing thing is the locks are still being explicitly
held while calling fireEvent anyways.

I will be attaching more details once ticket is created.

--
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

Mime
View raw message