Atmosphere/atmosphere — pull request #786 by rs017991 (2012-12-20 23:22:39+00:00)
Title: Fixed ConcurrentModification issues in AbstractBroadcasterCache

1.0.x

Funny, since we now have getQueueDepth(), I've been able to see the size of my caches, and have been puzzled to note that the sizes haven't gone down over time as expected.

Upon debugging, it is clear that any call to __queue.remove(message)__ at __AbstractBroadcasterCache:107__ will cause the subsequent __i.next()__ to throw a __ConcurrentModificationException__. Being thrown out of the Runnable, this exception is not logged and prevents all further executions of the reaper.

The end result of this is that the reaper is only capabable of removing one message, ever.

Was introduced in 7ba76848d9166210ce74d62b58b2498739b282b3 and therefore is present in Atmosphere 1.0.2 - 1.0.7 (Making 1.0.8 a critical patch for anybody using caches extensively in 1.0.x)

Keep in mind that this issue is __not__ due to any synchronization issue, but rather due to calling __remove__ on a List that is being iterated over, regardless of thread.

This itself can be fixed by changing __queue.remove(message)__ to __i.remove()__

However, there is still another bug in the reaper which causes a ConcurrentModificationException, one this time which does involve synchronization: The docs for synchronizedList indicate that the entire iteration must be wrapped in a synchronized block, otherwise modification in another thread will cause an error. See also:
* http://docs.oracle.com/javase/6/docs/api/java/util/Collections.html#synchronizedList(java.util.List)_

This is definitely the case here: broadcasts which occur during the reaping iteration will cause a ConcurrentModification, with the same effect of permanently killing the reaper process.

__I fixed this one by expanding the scope of the synchronized block.__