修复StampedCache的get方法非原子问题

CHANGELOG.md

@@ -2,7 +2,7 @@
 # 🚀Changelog
 
 -------------------------------------------------------------------------------------------------------------
-# 5.8.24(2023-12-05)
+# 5.8.24(2023-12-09)
 
 ### 🐣新特性
 * 【cache 】      Cache增加get重载，可自定义超时时间（issue#I8G0DL@Gitee）
@@ -16,6 +16,7 @@
 * 【http  】      修复RootAction send404 抛异常问题（pr#1107@Gitee）
 * 【extra 】      修复Archiver 最后一个 Entry 为空文件夹时未关闭 Entry问题（pr#1123@Gitee）
 * 【core  】      修复ImgUtil.convert png转jpg在jdk9+中失败问题（issue#I8L8UA@Gitee）
+* 【cache 】      修复StampedCache的get方法非原子问题（issue#I8MEIX@Gitee）
 
 -------------------------------------------------------------------------------------------------------------
 # 5.8.23(2023-11-12)

hutool-cache/src/main/java/cn/hutool/cache/impl/AbstractCache.java

@@ -254,16 +254,10 @@ public abstract class AbstractCache<K, V> implements Cache<K, V> {
 	 * 移除key对应的对象，不加锁
 	 *
 	 * @param key           键
-	 * @param withMissCount 是否计数丢失数
 	 * @return 移除的对象，无返回null
 	 */
-	protected CacheObj<K, V> removeWithoutLock(K key, boolean withMissCount) {
+	protected CacheObj<K, V> removeWithoutLock(K key) {
-		final CacheObj<K, V> co = cacheMap.remove(MutableObj.of(key));
+		return cacheMap.remove(MutableObj.of(key));
-		if (withMissCount) {
-			// 在丢失计数有效的情况下，移除一般为get时的超时操作，此处应该丢失数+1
-			this.missCount.increment();
-		}
-		return co;
 	}
 
 	/**

hutool-cache/src/main/java/cn/hutool/cache/impl/FIFOCache.java

@@ -71,7 +71,7 @@ public class FIFOCache<K, V> extends StampedCache<K, V> {
 
 		// 清理结束后依旧是满的，则删除第一个被缓存的对象
 		if (isFull() && null != first) {
-			removeWithoutLock(first.key, false);
+			removeWithoutLock(first.key);
 			onRemove(first.key, first.obj);
 			count++;
 		}

hutool-cache/src/main/java/cn/hutool/cache/impl/ReentrantCache.java

@@ -33,49 +33,12 @@ public abstract class ReentrantCache<K, V> extends AbstractCache<K, V> {
 
 	@Override
 	public boolean containsKey(K key) {
-		lock.lock();
+		return null != getOrRemoveExpired(key, false, false);
-		try {
-			// 不存在或已移除
-			final CacheObj<K, V> co = getWithoutLock(key);
-			if (co == null) {
-				return false;
-			}
-
-			if (false == co.isExpired()) {
-				// 命中
-				return true;
-			}
-		} finally {
-			lock.unlock();
-		}
-
-		// 过期
-		remove(key, true);
-		return false;
 	}
 
 	@Override
 	public V get(K key, boolean isUpdateLastAccess) {
-		CacheObj<K, V> co;
+		return getOrRemoveExpired(key, isUpdateLastAccess, true);
-		lock.lock();
-		try {
-			co = getWithoutLock(key);
-		} finally {
-			lock.unlock();
-		}
-
-		// 未命中
-		if (null == co) {
-			missCount.increment();
-			return null;
-		} else if (false == co.isExpired()) {
-			hitCount.increment();
-			return co.get(isUpdateLastAccess);
-		}
-
-		// 过期，既不算命中也不算非命中
-		remove(key, true);
-		return null;
 	}
 
 	@Override
@@ -102,7 +65,16 @@ public abstract class ReentrantCache<K, V> extends AbstractCache<K, V> {
 
 	@Override
 	public void remove(K key) {
-		remove(key, false);
+		lock.lock();
+		CacheObj<K, V> co;
+		try {
+			co = removeWithoutLock(key);
+		} finally {
+			lock.unlock();
+		}
+		if (null != co) {
+			onRemove(co.key, co.obj);
+		}
 	}
 
 	@Override
@@ -126,21 +98,37 @@ public abstract class ReentrantCache<K, V> extends AbstractCache<K, V> {
 	}
 
 	/**
-	 * 移除key对应的对象
+	 * 获得值或清除过期值
-	 *
+	 * @param key 键
-	 * @param key           键
+	 * @param isUpdateLastAccess 是否更新最后访问时间
-	 * @param withMissCount 是否计数丢失数
+	 * @param isUpdateCount 是否更新计数器
+	 * @return 值或null
 	 */
-	private void remove(K key, boolean withMissCount) {
+	private V getOrRemoveExpired(final K key, final boolean isUpdateLastAccess, final boolean isUpdateCount) {
-		lock.lock();
 		CacheObj<K, V> co;
+		lock.lock();
 		try {
-			co = removeWithoutLock(key, withMissCount);
+			co = getWithoutLock(key);
+			if(null != co && co.isExpired()){
+				//过期移除
+				removeWithoutLock(key);
+				co = null;
+			}
 		} finally {
 			lock.unlock();
 		}
-		if (null != co) {
+
-			onRemove(co.key, co.obj);
+		// 未命中
+		if (null == co) {
+			if(isUpdateCount){
+				missCount.increment();
+			}
+			return null;
 		}
+
+		if(isUpdateCount){
+			hitCount.increment();
+		}
+		return co.get(isUpdateLastAccess);
 	}
 }

hutool-cache/src/main/java/cn/hutool/cache/impl/StampedCache.java

@@ -1,6 +1,7 @@
 package cn.hutool.cache.impl;
 
 import cn.hutool.core.collection.CopiedIter;
+import cn.hutool.core.thread.ThreadUtil;
 
 import java.util.Iterator;
 import java.util.concurrent.locks.StampedLock;
@@ -13,7 +14,7 @@ import java.util.concurrent.locks.StampedLock;
  * @author looly
  * @since 5.7.15
  */
-public abstract class StampedCache<K, V> extends AbstractCache<K, V>{
+public abstract class StampedCache<K, V> extends AbstractCache<K, V> {
 	private static final long serialVersionUID = 1L;
 
 	// 乐观锁，此处使用乐观锁解决读多写少的场景
@@ -33,54 +34,12 @@ public abstract class StampedCache<K, V> extends AbstractCache<K, V>{
 
 	@Override
 	public boolean containsKey(K key) {
-		final long stamp = lock.readLock();
+		return null != get(key, false, false);
-		try {
-			// 不存在或已移除
-			final CacheObj<K, V> co = getWithoutLock(key);
-			if (co == null) {
-				return false;
-			}
-
-			if (false == co.isExpired()) {
-				// 命中
-				return true;
-			}
-		} finally {
-			lock.unlockRead(stamp);
-		}
-
-		// 过期
-		remove(key, true);
-		return false;
 	}
 
 	@Override
 	public V get(K key, boolean isUpdateLastAccess) {
-		// 尝试读取缓存，使用乐观读锁
+		return get(key, isUpdateLastAccess, true);
-		long stamp = lock.tryOptimisticRead();
-		CacheObj<K, V> co = getWithoutLock(key);
-		if(false == lock.validate(stamp)){
-			// 有写线程修改了此对象，悲观读
-			stamp = lock.readLock();
-			try {
-				co = getWithoutLock(key);
-			} finally {
-				lock.unlockRead(stamp);
-			}
-		}
-
-		// 未命中
-		if (null == co) {
-			missCount.increment();
-			return null;
-		} else if (false == co.isExpired()) {
-			hitCount.increment();
-			return co.get(isUpdateLastAccess);
-		}
-
-		// 过期，既不算命中也不算非命中
-		remove(key, true);
-		return null;
 	}
 
 	@Override
@@ -107,7 +66,16 @@ public abstract class StampedCache<K, V> extends AbstractCache<K, V>{
 
 	@Override
 	public void remove(K key) {
-		remove(key, false);
+		final long stamp = lock.writeLock();
+		CacheObj<K, V> co;
+		try {
+			co = removeWithoutLock(key);
+		} finally {
+			lock.unlockWrite(stamp);
+		}
+		if (null != co) {
+			onRemove(co.key, co.obj);
+		}
 	}
 
 	@Override
@@ -121,21 +89,75 @@ public abstract class StampedCache<K, V> extends AbstractCache<K, V>{
 	}
 
 	/**
-	 * 移除key对应的对象
+	 * 获取值
+	 *
+	 * @param key                键
+	 * @param isUpdateLastAccess 是否更新最后修改时间
+	 * @param isUpdateCount      是否更新命中数，get时更新，contains时不更新
+	 * @return 值或null
+	 */
+	private V get(K key, boolean isUpdateLastAccess, boolean isUpdateCount) {
+		// 尝试读取缓存，使用乐观读锁
+		long stamp = lock.tryOptimisticRead();
+		CacheObj<K, V> co = getWithoutLock(key);
+		if (false == lock.validate(stamp)) {
+			// 有写线程修改了此对象，悲观读
+			stamp = lock.readLock();
+			try {
+				co = getWithoutLock(key);
+			} finally {
+				lock.unlockRead(stamp);
+			}
+		}
+
+		// 未命中
+		if (null == co) {
+			if (isUpdateCount) {
+				missCount.increment();
+			}
+			return null;
+		} else if (false == co.isExpired()) {
+			if (isUpdateCount) {
+				hitCount.increment();
+			}
+			return co.get(isUpdateLastAccess);
+		}
+
+		// 悲观锁，二次检查
+		return getOrRemoveExpired(key, isUpdateCount);
+	}
+
+	/**
+	 * 同步获取值，如果过期则移除之
 	 *
 	 * @param key           键
-	 * @param withMissCount 是否计数丢失数
+	 * @param isUpdateCount 是否更新命中数，get时更新，contains时不更新
+	 * @return 有效值或null
 	 */
-	private void remove(K key, boolean withMissCount) {
+	private V getOrRemoveExpired(K key, boolean isUpdateCount) {
 		final long stamp = lock.writeLock();
 		CacheObj<K, V> co;
 		try {
-			co = removeWithoutLock(key, withMissCount);
+			co = getWithoutLock(key);
+			if (null == co) {
+				return null;
+			}
+			if (false == co.isExpired()) {
+				// 首先尝试获取值，如果值存在且有效，返回之
+				if (isUpdateCount) {
+					hitCount.increment();
+				}
+				return co.getValue();
+			}
+
+			// 无效移除
+			co = removeWithoutLock(key);
 		} finally {
 			lock.unlockWrite(stamp);
 		}
 		if (null != co) {
 			onRemove(co.key, co.obj);
 		}
+		return null;
 	}
 }

hutool-cache/src/test/java/cn/hutool/cache/IssueI8MEIXTest.java

@@ -0,0 +1,30 @@
+package cn.hutool.cache;
+
+import cn.hutool.cache.impl.TimedCache;
+import cn.hutool.core.lang.Console;
+import cn.hutool.core.thread.ThreadUtil;
+import org.junit.Ignore;
+import org.junit.Test;
+
+public class IssueI8MEIXTest {
+
+	@Test
+	@Ignore
+	public void getRemoveTest() {
+		final TimedCache<String, String> cache = new TimedCache<>(200);
+		cache.put("a", "123");
+
+		ThreadUtil.sleep(300);
+
+		// 测试时，在get后的remove前加sleep测试在读取过程中put新值的问题
+		ThreadUtil.execute(()->{
+			Console.log(cache.get("a"));
+		});
+
+		ThreadUtil.execute(()->{
+			cache.put("a", "456");
+		});
+
+		ThreadUtil.sleep(1000);
+	}
+}