Skip to content

Commit be40b08

Browse files
mp911dechristophstrobl
authored andcommitted
DATAREDIS-452 - Improve thread synchronization in RedisCacheTest.testCacheGetSynchronized.
Original Pull Request: spring-projects#165
1 parent 707005e commit be40b08

File tree

2 files changed

+61
-36
lines changed

2 files changed

+61
-36
lines changed

pom.xml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
<jedis>2.8.0</jedis>
2929
<srp>0.7</srp>
3030
<jredis>06052013</jredis>
31+
<multithreadedtc>1.01</multithreadedtc>
3132
</properties>
3233

3334
<dependencyManagement>
@@ -181,6 +182,13 @@
181182
<version>${xstream}</version>
182183
<scope>test</scope>
183184
</dependency>
185+
186+
<dependency>
187+
<groupId>edu.umd.cs.mtc</groupId>
188+
<artifactId>multithreadedtc</artifactId>
189+
<version>${multithreadedtc}</version>
190+
<scope>test</scope>
191+
</dependency>
184192
</dependencies>
185193

186194
<build>

src/test/java/org/springframework/data/redis/cache/RedisCacheTest.java

Lines changed: 53 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package org.springframework.data.redis.cache;
1818

19+
import static org.hamcrest.core.Is.is;
1920
import static org.hamcrest.core.IsEqual.*;
2021
import static org.hamcrest.core.IsInstanceOf.*;
2122
import static org.hamcrest.core.IsNot.*;
@@ -26,14 +27,13 @@
2627
import static org.springframework.data.redis.matcher.RedisTestMatchers.*;
2728

2829
import java.util.Collection;
29-
import java.util.List;
3030
import java.util.concurrent.Callable;
31-
import java.util.concurrent.CopyOnWriteArrayList;
3231
import java.util.concurrent.CountDownLatch;
3332
import java.util.concurrent.TimeUnit;
3433
import java.util.concurrent.atomic.AtomicBoolean;
35-
import java.util.concurrent.atomic.AtomicLong;
3634

35+
import edu.umd.cs.mtc.MultithreadedTestCase;
36+
import edu.umd.cs.mtc.TestFramework;
3737
import org.hamcrest.core.IsEqual;
3838
import org.junit.AfterClass;
3939
import org.junit.Before;
@@ -44,15 +44,16 @@
4444
import org.springframework.cache.Cache;
4545
import org.springframework.cache.Cache.ValueWrapper;
4646
import org.springframework.data.redis.ConnectionFactoryTracker;
47-
import org.springframework.data.redis.LongObjectFactory;
4847
import org.springframework.data.redis.ObjectFactory;
48+
import org.springframework.data.redis.StringObjectFactory;
4949
import org.springframework.data.redis.core.AbstractOperationsTestParams;
5050
import org.springframework.data.redis.core.RedisTemplate;
5151

5252
/**
5353
* @author Costin Leau
5454
* @author Jennifer Hickey
5555
* @author Christoph Strobl
56+
* @author Mark Paluch
5657
*/
5758
@SuppressWarnings("rawtypes")
5859
@RunWith(Parameterized.class)
@@ -283,50 +284,66 @@ public void putIfAbsentShouldSetValueOnlyIfNotPresent() {
283284

284285
/**
285286
* @see DATAREDIS-443
287+
* @see DATAREDIS-452
286288
*/
287289
@Test
288-
public void testCacheGetSynchronized() throws InterruptedException {
290+
public void testCacheGetSynchronized() throws Throwable {
289291

290292
assumeThat(cache, instanceOf(RedisCache.class));
291-
assumeThat(valueFactory, instanceOf(LongObjectFactory.class));
293+
assumeThat(valueFactory, instanceOf(StringObjectFactory.class));
292294

293-
int threadCount = 10;
294-
final AtomicLong counter = new AtomicLong();
295-
final List<Object> results = new CopyOnWriteArrayList<Object>();
296-
final CountDownLatch latch = new CountDownLatch(threadCount);
295+
TestFramework.runOnce(new CacheGetWithValueLoaderIsThreadSafe((RedisCache) cache));
296+
}
297297

298-
final RedisCache redisCache = (RedisCache) cache;
298+
static class CacheGetWithValueLoaderIsThreadSafe extends MultithreadedTestCase {
299299

300-
final Object key = getKey();
300+
RedisCache redisCache;
301+
TestCacheLoader<String> cacheLoader;
301302

302-
Runnable run = new Runnable() {
303-
@Override
304-
public void run() {
305-
try {
306-
Long value = redisCache.get(key, new Callable<Long>() {
307-
@Override
308-
public Long call() throws Exception {
309-
310-
Thread.sleep(333); // make sure the thread will overlap
311-
return counter.incrementAndGet();
312-
}
313-
});
314-
results.add(value);
315-
} finally {
316-
latch.countDown();
303+
public CacheGetWithValueLoaderIsThreadSafe(RedisCache redisCache) {
304+
this.redisCache = redisCache;
305+
306+
cacheLoader = new TestCacheLoader<String>("test"){
307+
308+
@Override
309+
public String call() throws Exception {
310+
waitForTick(2);
311+
return super.call();
317312
}
318-
}
319-
};
313+
};
314+
}
320315

321-
for (int i = 0; i < threadCount; i++) {
322-
new Thread(run).start();
323-
Thread.sleep(100);
316+
public void thread1(){
317+
assertTick(0);
318+
Thread.currentThread().setName(getClass().getSimpleName() + " Cache Loader Thread");
319+
320+
String result = redisCache.get("key", cacheLoader);
321+
322+
assertThat(result, is("test"));
324323
}
325-
latch.await();
326324

327-
assertThat(results.size(), IsEqual.equalTo(threadCount));
328-
for (Object result : results) {
329-
assertThat((Long) result, equalTo(1L));
325+
public void thread2(){
326+
waitForTick(1);
327+
Thread.currentThread().setName(getClass().getSimpleName() + " Cache Reader Thread");
328+
329+
String result = redisCache.get("key", new TestCacheLoader<String>("illegal value"));
330+
331+
assertThat(result, is("test"));
332+
assertTick(2);
330333
}
331334
}
335+
336+
protected static class TestCacheLoader<T> implements Callable<T> {
337+
338+
private final T value;
339+
340+
public TestCacheLoader(T value) {
341+
this.value = value;
342+
}
343+
344+
@Override
345+
public T call() throws Exception {
346+
return value;
347+
}
348+
}
332349
}

0 commit comments

Comments
 (0)