Throw exception when a nested call to MemoryCache.Get<T>() is made with identical keys to prevent deadlocks.

muratcakir · muratcakir · commit b8b3b6ba7376 · 2019-05-03T03:10:18.000+02:00
diff --git a/src/Libraries/SmartStore.Core/Caching/MemoryCacheManager.cs b/src/Libraries/SmartStore.Core/Caching/MemoryCacheManager.cs
@@ -15,6 +15,8 @@ namespace SmartStore.Core.Caching
 {
     public partial class MemoryCacheManager : DisposableObject, ICacheManager
     {
+		const string LockRecursionExceptionMessage = "Acquiring identical cache items recursively is not supported. Key: {0}";
+		
 		// Wwe put a special string into cache if value is null,
 		// otherwise our 'Contains()' would always return false,
 		// which is bad if we intentionally wanted to save NULL values.
@@ -79,8 +81,14 @@ public T Get<T>(string key, Func<T> acquirer, TimeSpan? duration = null, bool in
 				return value;
 			}
 
+			var lockKey = "cache:" + key;
+			if (KeyedLock.IsLockHeld(lockKey))
+			{
+				throw new LockRecursionException(LockRecursionExceptionMessage.FormatInvariant(key));
+			}
+
 			// Get the (semaphore) locker specific to this key
-			using (KeyedLock.Lock(key))
+			using (KeyedLock.Lock(lockKey, TimeSpan.FromMinutes(1)))
 			{
 				// Atomic operation must be outer locked
 				if (!TryGet(key, independent, out value))
@@ -104,8 +112,14 @@ public async Task<T> GetAsync<T>(string key, Func<Task<T>> acquirer, TimeSpan? d
 				return value;
 			}
 
+			var lockKey = "cache:" + key;
+			if (KeyedLock.IsLockHeld(lockKey))
+			{
+				throw new LockRecursionException(LockRecursionExceptionMessage.FormatInvariant(key));
+			}
+
 			// Get the async (semaphore) locker specific to this key
-			using (await KeyedLock.LockAsync(key))
+			using (await KeyedLock.LockAsync(lockKey, TimeSpan.FromMinutes(1)))
 			{
 				if (!TryGet(key, independent, out value))
 				{
diff --git a/src/Libraries/SmartStore.Core/Utilities/Threading/KeyedLock.cs b/src/Libraries/SmartStore.Core/Utilities/Threading/KeyedLock.cs
@@ -22,7 +22,7 @@ private KeyedLock(object key)
 		private object Key { get; set; }
 		private SemaphoreSlim Semaphore { get; set; }
 
-		public bool IsLockHeld
+		public bool HasWaiters
 		{
 			get { return _waiterCount > 1; }
 		}
@@ -38,6 +38,11 @@ public int WaiterCount
 		[MethodImpl(MethodImplOptions.AggressiveInlining)]
 		public void DecrementCount() => Interlocked.Decrement(ref _waiterCount);
 
+		public static bool IsLockHeld(object key)
+		{
+			return _locks.ContainsKey(key);
+		}
+
 		public static IDisposable Lock(object key)
 		{
 			return Lock(key, _noTimeout);

Original file line number	Diff line number	Diff line change
`@@ -15,6 +15,8 @@ namespace SmartStore.Core.Caching`
`15`	`15`	`{`
`16`	`16`	`public partial class MemoryCacheManager : DisposableObject, ICacheManager`
`17`	`17`	`{`
	`18`	`+ const string LockRecursionExceptionMessage = "Acquiring identical cache items recursively is not supported. Key: {0}";`
	`19`	`+`
`18`	`20`	`// Wwe put a special string into cache if value is null,`
`19`	`21`	`// otherwise our 'Contains()' would always return false,`
`20`	`22`	`// which is bad if we intentionally wanted to save NULL values.`
`@@ -79,8 +81,14 @@ public T Get<T>(string key, Func<T> acquirer, TimeSpan? duration = null, bool in`
`79`	`81`	`return value;`
`80`	`82`	`}`
`81`	`83`
	`84`	`+ var lockKey = "cache:" + key;`
	`85`	`+ if (KeyedLock.IsLockHeld(lockKey))`
	`86`	`+ {`
	`87`	`+ throw new LockRecursionException(LockRecursionExceptionMessage.FormatInvariant(key));`
	`88`	`+ }`
	`89`	`+`
`82`	`90`	`// Get the (semaphore) locker specific to this key`
`83`		`- using (KeyedLock.Lock(key))`
	`91`	`+ using (KeyedLock.Lock(lockKey, TimeSpan.FromMinutes(1)))`
`84`	`92`	`{`
`85`	`93`	`// Atomic operation must be outer locked`
`86`	`94`	`if (!TryGet(key, independent, out value))`
`@@ -104,8 +112,14 @@ public async Task<T> GetAsync<T>(string key, Func<Task<T>> acquirer, TimeSpan? d`
`104`	`112`	`return value;`
`105`	`113`	`}`
`106`	`114`
	`115`	`+ var lockKey = "cache:" + key;`
	`116`	`+ if (KeyedLock.IsLockHeld(lockKey))`
	`117`	`+ {`
	`118`	`+ throw new LockRecursionException(LockRecursionExceptionMessage.FormatInvariant(key));`
	`119`	`+ }`
	`120`	`+`
`107`	`121`	`// Get the async (semaphore) locker specific to this key`
`108`		`- using (await KeyedLock.LockAsync(key))`
	`122`	`+ using (await KeyedLock.LockAsync(lockKey, TimeSpan.FromMinutes(1)))`
`109`	`123`	`{`
`110`	`124`	`if (!TryGet(key, independent, out value))`
`111`	`125`	`{`
Original file line number	Diff line number	Diff line change
`@@ -22,7 +22,7 @@ private KeyedLock(object key)`
`22`	`22`	`private object Key { get; set; }`
`23`	`23`	`private SemaphoreSlim Semaphore { get; set; }`
`24`	`24`
`25`		`- public bool IsLockHeld`
	`25`	`+ public bool HasWaiters`
`26`	`26`	`{`
`27`	`27`	`get { return _waiterCount > 1; }`
`28`	`28`	`}`
`@@ -38,6 +38,11 @@ public int WaiterCount`
`38`	`38`	`[MethodImpl(MethodImplOptions.AggressiveInlining)]`
`39`	`39`	`public void DecrementCount() => Interlocked.Decrement(ref _waiterCount);`
`40`	`40`
	`41`	`+ public static bool IsLockHeld(object key)`
	`42`	`+ {`
	`43`	`+ return _locks.ContainsKey(key);`
	`44`	`+ }`
	`45`	`+`
`41`	`46`	`public static IDisposable Lock(object key)`
`42`	`47`	`{`
`43`	`48`	`return Lock(key, _noTimeout);`