dotnet / dotnet/docs

The CacheSignal<T> class does not work as described in the article due to its usage of SemaphoreSlim

Open
#50,562 0 comments 0 reactions 0 assignees View on GitHub
:watch: Not Triaged dotnet-fundamentals/svc
Dominant language
No language data
Stars
4.8k
Forks
6.1k
Avg merge
15h 21m
Merged PRs (30d)
370

Description

### Type of issue

Other (describe below)

### Description

The class `CacheSignal` should signal that the cache is populated before accessing it.

> Calls and waits for the _cacheSignal.WaitAsync() to release, this ensures that the cache is populated before accessing the cache.

The semaphore of the CacheSignal is already released in its initial state, it should be created with `new (0, 1)` instead of `new (1, 1)` otherwise the `PhotoService` may access an unpopulated cache.

```csharp
namespace CachingExamples.Memory;

public sealed class CacheSignal
{
private readonly SemaphoreSlim _semaphore = new(1, 1);

///
/// Exposes a that represents the asynchronous wait operation.
/// When signaled (consumer calls ), the
/// is set as .
///
public Task WaitAsync() => _semaphore.WaitAsync();

///
/// Exposes the ability to signal the release of the 's operation.
/// Callers who were waiting, will be able to continue.
///
public void Release() => _semaphore.Release();
}
```

**Suggestion**

Additionally I would suggest a refactor of the `CacheSignal` as it should only be released from the `CacheWorker` once the cache is populated. The `PhotoService` should not need that mutual access to the cache.

`CacheSignal` should utilize a `TaskCompletionSource` instead of a `SemaphoreSlim`.

```csharp
namespace CachingExamples.Memory;

public sealed class CacheSignal
{
private readonly TaskCompletionSource _tcs = new();

///
/// Exposes a that represents the asynchronous wait operation for the population of the cache.
/// When signaled (consumer calls ), the
/// is set as .
///
public Task WaitForPopulateAsync() => _tcs.Task;

///
/// Exposes the ability to signal the release of the 's operation to indicate that the cache has been populated.
/// This only needs to be called once after population of the cache.
/// Callers who were waiting, will be able to continue.
///
public void Release() => _tcs.TrySetResult();
}
```

The `PhotoService` should just wait for the population of the cache and not release it anymore. The cache itself is thread safe, therefore there is no need for the mutual access due to the previous behavior of the `CacheSignal`.

```csharp
using Microsoft.Extensions.Caching.Memory;

namespace CachingExamples.Memory;

public sealed class PhotoService(
IMemoryCache cache,
CacheSignal cacheSignal,
ILogger logger)
{
public async IAsyncEnumerable GetPhotosAsync(Func? filter = default)
{
await cacheSignal.WaitForPopulateAsync(); // wait until the cache is populated

Photo[] photos =
(await cache.GetOrCreateAsync(
"Photos", _ =>
{
logger.LogWarning("This should never happen!");

return Task.FromResult(Array.Empty());
}))!;

// If no filter is provided, use a pass-thru.
filter ??= _ => true;

foreach (Photo photo in photos)
{
if (!default(Photo).Equals(photo) && filter(photo))
{
yield return photo;
}
}
}
}
```

### Page URL

https://learn.microsoft.com/en-us/dotnet/core/extensions/caching

### Content source URL

https://github.com/dotnet/docs/blob/main/docs/core/extensions/caching.md

### Document Version Independent Id

07beacc6-ddcf-2fd5-b896-00f999764b00

### Platform Id

97e770aa-0b26-d69d-80e4-578056090645

### Article author

@IEvangelist

### Metadata

* ID: ed120c7a-f4c0-c1d1-cb8c-168a3eba6dd9
* PlatformId: 97e770aa-0b26-d69d-80e4-578056090645
* Service: **dotnet-fundamentals**

[Related Issues](https://github.com/dotnet/docs/issues?q=is%3Aissue+is%3Aopen+07beacc6-ddcf-2fd5-b896-00f999764b00)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.