OPEN - issue BATCH-1156: Possible "null" context return with MapRetryContextCache in RetryTemplate

Check for null reference before deciding that a key exists.

spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/policy/MapRetryContextCache.java

@@ -24,7 +24,8 @@ import org.springframework.batch.retry.RetryContext;
 
 /**
  * Map-based implementation of {@link RetryContextCache}. The map backing the
- * cache of contexts is synchronized and its entries are soft-referenced.  
+ * cache of contexts is synchronized and its entries are soft-referenced, so may
+ * be garbage collected under pressure.
  * 
  * @author Dave Syer
  * 
@@ -37,8 +38,9 @@ public class MapRetryContextCache implements RetryContextCache {
 	 * cache with item keys that are inconsistent.
 	 */
 	public static final int DEFAULT_CAPACITY = 4096;
-	
-	private Map<Object, SoftReference<RetryContext>> map = Collections.synchronizedMap(new HashMap<Object, SoftReference<RetryContext>>());
+
+	private Map<Object, SoftReference<RetryContext>> map = Collections
+			.synchronizedMap(new HashMap<Object, SoftReference<RetryContext>>());
 
 	private int capacity;
 
@@ -71,6 +73,13 @@ public class MapRetryContextCache implements RetryContextCache {
 	}
 
 	public boolean containsKey(Object key) {
+		if (!map.containsKey(key)) {
+			return false;
+		}
+		if (map.get(key).get() == null) {
+			// our reference was garbage collected
+			map.remove(key);
+		}
 		return map.containsKey(key);
 	}
 
@@ -80,7 +89,9 @@ public class MapRetryContextCache implements RetryContextCache {
 
 	public void put(Object key, RetryContext context) {
 		if (map.size() >= capacity) {
-			throw new RetryCacheCapacityExceededException("Retry cache capacity limit breached. " + "Do you need to re-consider the implementation of the key generator, " + "or the equals and hashCode of the items that failed?");
+			throw new RetryCacheCapacityExceededException("Retry cache capacity limit breached. "
+					+ "Do you need to re-consider the implementation of the key generator, "
+					+ "or the equals and hashCode of the items that failed?");
 		}
 		map.put(key, new SoftReference<RetryContext>(context));
 	}

spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/support/RetryTemplate.java

@@ -236,7 +236,7 @@ public class RetryTemplate implements RetryOperations {
 					doOnErrorInterceptors(retryCallback, context, e);
 
 					registerThrowable(retryPolicy, state, context, e);
-					
+
 					try {
 						backOffPolicy.backOff(backOffContext);
 					}
@@ -247,7 +247,7 @@ public class RetryTemplate implements RetryOperations {
 						logger.debug("Abort retry because interrupted: count=" + context.getRetryCount());
 						throw ex;
 					}
-					
+
 					if (shouldRethrow(retryPolicy, context, state)) {
 						logger.debug("Rethrow in retry for policy: count=" + context.getRetryCount());
 						throw e;
@@ -350,24 +350,28 @@ public class RetryTemplate implements RetryOperations {
 		if (state.isForceRefresh()) {
 			return doOpenInternal(retryPolicy);
 		}
-		else if (retryContextCache.containsKey(key)) {
 
-			RetryContext context = retryContextCache.get(key);
-			if (context == null) {
+		// If there is no cache hit we can avoid the possible expense of the
+		// cache re-hydration.
+		if (!retryContextCache.containsKey(key)) {
+			// The cache is only used if there is a failure.
+			return doOpenInternal(retryPolicy);
+		}
+
+		RetryContext context = retryContextCache.get(key);
+		if (context == null) {
+			if (retryContextCache.containsKey(key)) {
 				throw new RetryException("Inconsistent state for failed item: no history found. "
 						+ "Consider whether equals() or hashCode() for the item might be inconsistent, "
 						+ "or if you need to supply a better ItemKeyGenerator");
 			}
-			return context;
-
-		}
-		else {
-
-			// The cache is only used if there is a failure.
+			// The cache could have been expired in between calls to
+			// containsKey(), so we have to live with this:
 			return doOpenInternal(retryPolicy);
-
 		}
 
+		return context;
+
 	}
 
 	/**