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
