Race condition when updating a value's weight

[GoogleCodeExporter]

> What steps will reproduce the problem?
1. Use non-singleton weigher (values > 1)
2. Change a value's weight, re-put.
3. Create enough load to simultaneously evict the value in (2).

> What is the expected output? What do you see instead?
A NPE is thrown during the eviction due to the map overflowing its capacity, 
but with no more nodes in the LRU chain to evict.

> Cause:
The problem is due to a stale volatile read of the node's weight in #evict(). 
While the entry is no longer in the data map (CHM), it may still be mutated in 
#put() using the decorator's segment lock.

> Solution:
Update #evict() to read the weight under the segment lock, thereby forcing any 
mutations to have completed and that the node is not referenced.

Lock lock = segmentLock[segmentFor(key)];
lock.lock();
try {
  weightedSize -= node.weightedValue.weight;
} finally {
lock.unlock();

Reference:
See discussion thread at:
http://groups.google.com/group/concurrentlinkedhashmap/browse_thread/thread/5b4b
3df843efd138

Original issue reported on code.google.com by Ben.Manes@gmail.com on 21 Oct 2010 at 8:33

[GoogleCodeExporter]

I plan on fixing and releasing an update over the weekend.

As its a good excuse to get back to v1.1 (which has been on the backburner due 
to MapMaker / other work), I'll try to complete that effort and bundle it all 
in one release cycle.

Original comment by Ben.Manes@gmail.com on 21 Oct 2010 at 8:36

[GoogleCodeExporter]

Thanks Ben for taking care of this.
I'm trying to come up with a test case for it.

Original comment by patric...@gmail.com on 21 Oct 2010 at 10:23

[GoogleCodeExporter]

This test should go green if it can catch the NullPointerException generated 
due to the race condition during the eviction.

After changing the code as Ben suggested, I don't see the exception anymore 
after running the tests several times. 

Original comment by patric...@gmail.com on 22 Oct 2010 at 12:11

Attachments:

• EvictRaceConditionTest.java

[GoogleCodeExporter]

Fixed in r471, please code review.

I plan on resolving the other pending issues tomorrow and releasing v1.1 
shortly afterwards.

Original comment by Ben.Manes@gmail.com on 24 Oct 2010 at 1:40

• Changed state: Fixed