Better synchronization in ConcurrentMapCache

Issue: SPR-13810

Author: snicoll

spring-context/src/main/java/org/springframework/cache/concurrent/ConcurrentMapCache.java

@@ -59,7 +59,7 @@ public class ConcurrentMapCache extends AbstractValueAdaptingCache {
 	 * @param name the name of the cache
 	 */
 	public ConcurrentMapCache(String name) {
-		this(name, new ConcurrentHashMap<Object, Object>(256), true);
+		this(name, new ConcurrentHashMap<>(256), true);
 	}
 
 	/**
@@ -69,7 +69,7 @@ public ConcurrentMapCache(String name) {
 	 * values for this cache
 	 */
 	public ConcurrentMapCache(String name, boolean allowNullValues) {
-		this(name, new ConcurrentHashMap<Object, Object>(256), allowNullValues);
+		this(name, new ConcurrentHashMap<>(256), allowNullValues);
 	}
 
 	/**
@@ -141,20 +141,14 @@ public <T> T get(Object key, Callable<T> valueLoader) {
 			return (T) get(key).get();
 		}
 		else {
-			synchronized (this.store) {
-				if (this.store.containsKey(key)) {
-					return (T) get(key).get();
-				}
-				T value;
+			return (T) fromStoreValue(this.store.computeIfAbsent(key, r -> {
 				try {
-					value = valueLoader.call();
+					return toStoreValue(valueLoader.call());
 				}
 				catch (Exception ex) {
 					throw new ValueRetrievalException(key, valueLoader, ex);
 				}
-				put(key, value);
-				return value;
-			}
+			}));
 		}
 	}
 

spring-context/src/test/java/org/springframework/cache/concurrent/ConcurrentMapCacheTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2016 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -44,9 +44,9 @@ public class ConcurrentMapCacheTests extends AbstractCacheTests<ConcurrentMapCac
 
 	@Before
 	public void setUp() throws Exception {
-		nativeCache = new ConcurrentHashMap<Object, Object>();
-		cache = new ConcurrentMapCache(CACHE_NAME, nativeCache, true);
-		cache.clear();
+		this.nativeCache = new ConcurrentHashMap<>();
+		this.cache = new ConcurrentMapCache(CACHE_NAME, this.nativeCache, true);
+		this.cache.clear();
 	}
 
 	@Override
@@ -84,9 +84,9 @@ public void testSerializer() {
 	public void testNonSerializableContent() {
 		ConcurrentMapCache serializeCache = createCacheWithStoreByValue();
 
-		thrown.expect(IllegalArgumentException.class);
-		thrown.expectMessage("Failed to serialize");
-		thrown.expectMessage(this.cache.getClass().getName());
+		this.thrown.expect(IllegalArgumentException.class);
+		this.thrown.expectMessage("Failed to serialize");
+		this.thrown.expectMessage(this.cache.getClass().getName());
 		serializeCache.put(createRandomKey(), this.cache);
 	}
 
@@ -96,15 +96,15 @@ public void testInvalidSerializedContent() {
 
 		String key = createRandomKey();
 		this.nativeCache.put(key, "Some garbage");
-		thrown.expect(IllegalArgumentException.class);
-		thrown.expectMessage("Failed to deserialize");
-		thrown.expectMessage("Some garbage");
+		this.thrown.expect(IllegalArgumentException.class);
+		this.thrown.expectMessage("Failed to deserialize");
+		this.thrown.expectMessage("Some garbage");
 		serializeCache.get(key);
 	}
 
 
 	private ConcurrentMapCache createCacheWithStoreByValue() {
-		return new ConcurrentMapCache(CACHE_NAME, nativeCache, true,
+		return new ConcurrentMapCache(CACHE_NAME, this.nativeCache, true,
 				new SerializationDelegate(ConcurrentMapCacheTests.class.getClassLoader()));
 	}