November 28, 2017
The class org.springframework.util.ConcurrentReferenceHashMap which is part of the spring framework is not thread-safe. We see this in the following junit test:
@RunWith(ConcurrentTestRunner.class) public class TestSpringConcurrentReferenceHashMap { private ConcurrentReferenceHashMap map = new ConcurrentReferenceHashMap(); private KeyAndValue[] keyAndValue; private static final int SIZE = 100; public TestSpringConcurrentReferenceHashMap() { keyAndValue = new KeyAndValue[SIZE]; for(int i = 0 ; i< SIZE ; i++) { keyAndValue[i] = new KeyAndValue(i); } } @Test public void test() { for(int i = 0 ; i < SIZE ; i++) { AtomicInteger result = (AtomicInteger)map.get(i); if( result == null ) { result = (AtomicInteger) map.putIfAbsent( keyAndValue[i].key , keyAndValue[i].value ); } if( result != null ) { result.addAndGet(1); } } } class KeyAndValue { KeyAndValue(int key) { this.key = key; value= new AtomicInteger(); } Integer key; AtomicInteger value; } }
The ConcurrentTestRunner runs the test by 4 threads in parallel. vmlens, a tool to detect deadlocks and race conditions during the test run, shows the following race conditions:
For example, the first race is the access to a value of an array without correct synchronization. In the method getReference the value of the array is read in line 12
@Nullable public Reference<K, V> getReference(@Nullable Object key, int hash, Restructure restructure) { if (restructure == Restructure.WHEN_NECESSARY) { restructureIfNecessary(false); } if (this.count == 0) { return null; } // Use a local copy to protect against other threads writing Reference<K, V>[] references = this.references; int index = getIndex(hash, references); Reference<K, V> head = references[index]; return findInChain(head, key, hash); }
And in the method add the value of the array is written in line 22.
@Nullable public <T> T doTask(final int hash, final Object key, final Task<T> task) { boolean resize = task.hasOption(TaskOption.RESIZE); if (task.hasOption(TaskOption.RESTRUCTURE_BEFORE)) { restructureIfNecessary(resize); } if (task.hasOption(TaskOption.SKIP_IF_EMPTY) && this.count == 0) { return task.execute(null, null, null); } lock(); try { final int index = getIndex(hash, this.references); final Reference<K, V> head = this.references[index]; Reference<K, V> reference = findInChain(head, key, hash); Entry<K, V> entry = (reference != null ? reference.get() : null); Entries entries = new Entries() { @Override public void add(V value) { @SuppressWarnings("unchecked") Entry<K, V> newEntry = new Entry<>((K) key, value); Reference<K, V> newReference = Segment.this.referenceManager.createReference(newEntry, hash, head); Segment.this.references[index] = newReference; Segment.this.count++; } }; return task.execute(reference, entry, entries); } finally { unlock(); if (task.hasOption(TaskOption.RESTRUCTURE_AFTER)) { restructureIfNecessary(resize); } } }
The field references is declared as volatile. But the access to the value of the array references[index] is not volatile. To make the access to an array volatile we need to use java.util.concurrent.atomic.AtomicReferenceArray.
To see the consequence of the race conditions we use jcstress, an open JDK code tool: The Java Concurrency Stress tests (jcstress) is an experimental harness and a suite of tests to aid the research in the correctness of concurrency support in the JVM, class libraries, and hardware.
I use the following test class:
@JCStressTest @Outcome(id = "10", expect = Expect.ACCEPTABLE, desc = "Default outcome.") @State public class SpringConcurrentHashMap { private final ConcurrentReferenceHashMap map = new ConcurrentReferenceHashMap(); private final KeyAndValue[] keyAndValue; private final int SIZE = 10; class KeyAndValue { KeyAndValue(int key) { this.key = key; value = new AtomicInteger(); } final Integer key; final AtomicInteger value; } public SpringConcurrentHashMap() { keyAndValue = new KeyAndValue[SIZE]; for (int i = 0; i < SIZE; i++) { keyAndValue[i] = new KeyAndValue(i); } } public void test() { for (int i = 0; i < SIZE; i++) { AtomicInteger result = (AtomicInteger) map.get(i); if (result == null) { result = (AtomicInteger) map.putIfAbsent(keyAndValue[i].key, keyAndValue[i].value); } if (result != null) { result.addAndGet(1); } } } @Actor public void actor1() { test(); } @Actor public void actor2() { test(); } @Arbiter public void arbiter(IntResult1 r) { int sum = 0; for (int i = 0; i < SIZE; i++) { sum += ((AtomicInteger) map.get(i)).get(); } r.r1 = sum; } }
The jcstress tool runs each actor in a separate thread, repeating the test multiple times. We have two actor method, each creating a new AtomicInteger in the hash map if no value is there or incrementing the AtomicInteger if a value existed. Therefore we expect the sum for all values in the hash map after the thread was run 10. If I run this test using the stress mode on my development machine, an intel i5 4 core CPU with oracle JDK 8, I see the following result:
2 matching test results. [FAILED] com.vmlens.stressTest.tests.SpringConcurrentHashMap (JVM args: [-client]) Observed state Occurrences Expectation Interpretation 10 92,597,143 ACCEPTABLE Default outcome. 9 7 FORBIDDEN No default case provided, assume FORBIDDEN [FAILED] com.vmlens.stressTest.tests.SpringConcurrentHashMap (JVM args: [-server]) Observed state Occurrences Expectation Interpretation 10 98,517,611 ACCEPTABLE Default outcome. 9 9 FORBIDDEN No default case provided, assume FORBIDDEN
In a small percentage of all cases, 7 cases for -client and 9 cases for -server the count is less than 10. So in some cases, a value in the map is overridden by a new value.
© 2020 vmlens Legal Notice Privacy Policy