Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix race condition in CacheEntryKeyLock.WaitAsync #282

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 7 additions & 16 deletions src/CacheTower/CacheStack.cs
Original file line number Diff line number Diff line change
Expand Up @@ -329,8 +329,14 @@ private async ValueTask BackPopulateCacheAsync<T>(int fromIndexExclusive, string

private async ValueTask<CacheEntry<T>?> RefreshValueAsync<T>(string cacheKey, Func<T, Task<T>> asyncValueFactory, CacheSettings settings, CacheEntryStatus entryStatus)
{
if (KeyLock.AcquireLock(cacheKey))
if (KeyLock.TryAcquireLockOrWaitAsync(cacheKey) is Task<ICacheEntry> task)
{
//We lost race to refresh
return await task as CacheEntry<T>;
}
else
{
// we got lock
try
{
var previousEntry = await GetAsync<T>(cacheKey);
Expand Down Expand Up @@ -374,21 +380,6 @@ private async ValueTask BackPopulateCacheAsync<T>(int fromIndexExclusive, string
throw;
}
}
else if (entryStatus != CacheEntryStatus.Stale)
{
//Last minute check to confirm whether waiting is required
var currentEntry = await GetAsync<T>(cacheKey);
if (currentEntry != null && currentEntry.GetStaleDate(settings) > DateTimeProvider.Now)
{
KeyLock.ReleaseLock(cacheKey, currentEntry);
return currentEntry;
}

//Lock until we are notified to be unlocked
return await KeyLock.WaitAsync(cacheKey) as CacheEntry<T>;
}

return default;
}

/// <summary>
Expand Down
36 changes: 24 additions & 12 deletions src/CacheTower/Internal/CacheEntryKeyLock.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,33 +15,45 @@ public bool AcquireLock(string cacheKey)
{
lock (keyLocks)
{
return TryAddImpl(cacheKey, null);
}
}

private bool TryAddImpl (string cacheKey, TaskCompletionSource<ICacheEntry>? value)
{
#if NETSTANDARD2_0
var hasLock = !keyLocks.ContainsKey(cacheKey);
if (hasLock)
var needToAdd = !keyLocks.ContainsKey(cacheKey);
if (needToAdd)
{
keyLocks[cacheKey] = null;
keyLocks[cacheKey] = value;
}
return hasLock;
return needToAdd;
#elif NETSTANDARD2_1
return keyLocks.TryAdd(cacheKey, null);
return keyLocks.TryAdd(cacheKey, value);
#endif
}
}

public Task<ICacheEntry> WaitAsync(string cacheKey)
/// <summary>
/// Returns null if acquire lock succeeded or returns task that wait until another thread will release lock and publish value.
///
/// </summary>
public Task<ICacheEntry>? TryAcquireLockOrWaitAsync(string cacheKey)
{
TaskCompletionSource<ICacheEntry>? completionSource;

lock (keyLocks)
{
if (!keyLocks.TryGetValue(cacheKey, out completionSource) || completionSource == null)
if (TryAddImpl(cacheKey, null))
{
// Lock acquired
return null;
}
var completionSource = keyLocks[cacheKey]; // Always exists here
if (completionSource == null)
{
completionSource = new TaskCompletionSource<ICacheEntry>();
keyLocks[cacheKey] = completionSource;
}
return completionSource.Task;
}

return completionSource.Task;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
Expand Down
Loading