HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Caching a value just once and update it without blocking other threads

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
oncewithoutupdatejustthreadscachingvalueblockingandother

Problem

I need some advice about my code.
The following code will create a new object the first time, and also force other threads to wait for the creation. When the object is created, we return the cached value, and when the value is about to expire, we renew it without blocking other threads, the reference is replaced by the new value.

I'm not sure about the SemaphoreSlim & ManualResetEvet parts, may be somebody can simplify this code.

Thanks.

```
public class UpdatableLazy
{
public class Container
{
public readonly T Value;

internal Container(T value)
{
Value = value;
}
}

private readonly Func> _updateFunc;
private readonly Func _isRenewNeeded;
private volatile Container _container;
private volatile bool _isUpdating;
private readonly SemaphoreSlim _ss = new SemaphoreSlim(1, 1);

public UpdatableLazy(Func> updateFunc, Func isRenewNeeded)
{
_updateFunc = updateFunc;
_isRenewNeeded = isRenewNeeded;
}

private readonly ManualResetEventSlim _mre = new ManualResetEventSlim();

public async Task GetValueAsync()
{
//The value doesn't exist yet
if (_container == null)
{
//if a thread is already creating the value, we will wait for the value
if (!await _ss.WaitAsync(0).ConfigureAwait(false))
{
_mre.Wait();
}

//After the release of the ManualResetEvent, the value should be available
if (_container != null)
{
return _container.Value;
}

try
{
//Let's create the value
_container = new Container(await _updateFunc().ConfigureAwait(false));
return _container.Value;
}
finally
{
//We tell awaiting threads that the value is available
_mre.Set();
_ss.Release();

Solution

The initial creation looks thread-safe at first glance. I don't know why you await your semaphore instead of just waiting synchronously with _ss.Wait(0) but I don't think it matters much.

The updating part however is not thread-safe. Any number of threads can come in between:

if (!_isUpdating && _isRenewNeeded(_container.Value))


and

_isUpdating = true;


This will result in every thread getting their own value, which is probably not what you want.

You should also call Dispose on disposable objects when you no longer need them.

Code Snippets

if (!_isUpdating && _isRenewNeeded(_container.Value))
_isUpdating = true;

Context

StackExchange Code Review Q#162279, answer score: 2

Revisions (0)

No revisions yet.