CODEWITHSUNDEEP
c#multithreadingasp.net-core.net-core
JensB
JensB

Reputation: 6880

Are static variables thread safe in the sense that you can read/write from multiple threads without it crashing?

I have a property defined as:

private static MyStaticCache? _myStaticCache;

MyStaticCache is a class with a string property. Multiple threads can access _myStaticCache. If the property is null or the value is old any thread accessing it will get the value from the source and set _myStaticCache to that value.

public string GetValue()
{
    if (_myStaticCache == null || _myStaticCache.CacheIsStale())
        _myStaticCache = GetValueFromSource();

    return _myStaticCache.Value1;
}

No code can or will ever sets _myStaticCache back to null.

One can quickly see that it's possible for multiple calls to GetValueFromSource() and assignments to _myStaticCache if multiple threads run before it is assigned the first time or when it has gone stale.

My question is this. Is there a way that this will cause a crash? Is the assignment of _myStaticCache atomic, or could there be a read in the middle of a write?

The fact that the the source method could be called N times in parallel does not really matter. The timeout for the cache is 30 days, and it is very unlikely that multiple threads will run at that exact time although not impossible, and even if there would be 100 threads running in parallel calling it the method would return the same value for each one and be able to handle the load no problem.

Now I could use a Mutex, or wrap the read and write in lock() but I'm thinking this would hinder performance for the 99.999% of the time this method is being called, again it will only be null or old every 30 days.

Upvotes: 3

Views: 1013

Answers (2)

Boppity Bop
Boppity Bop

Reputation: 10483

You really need to use synchronization (I simply dont believe you have a case for not using it, judging by the question). Even ints and other values types often require some sort of synchronization (eg memory barrier) despite being touted "atomic".

What I would do in your case is to simply wrap all access to your shared object with ReaderWriterLockSlim (and also Id suggest to have a singleton instead of static)

https://learn.microsoft.com/en-us/dotnet/api/system.threading.readerwriterlockslim?view=net-5.0

private ReaderWriterLockSlim cacheLock = new ReaderWriterLockSlim();

    public MyValueType ReadValue()
    {
        cacheLock.EnterReadLock();
        try
        {
            return myCache.Value1;
        }
        finally
        {
            cacheLock.ExitReadLock();
        }
    }

    public void WriteValue(MyValueType value)
    {
        cacheLock.EnterWriteLock();
        try
        {
            myCache.Value1 = value;
        }
        finally
        {
            cacheLock.ExitWriteLock();
        }
    }

Upvotes: -1

Marc Gravell
Marc Gravell

Reputation: 1064114

As long as MyStaticCache is a class (or interface; basically: a reference-type), you should be fine.

The language specification guarantees that you won't ever get torn references, so no it shouldn't ever crash:

12.5 Atomicity of variable references

Reads and writes of the following data types shall be atomic: bool, char, byte, sbyte, short, ushort, uint, int, float, and reference types. In addition, reads and writes of enum types with an underlying type in the previous list shall also be atomic. Reads and writes of other types, including long, ulong, double, and decimal, as well as user-defined types, need not be atomic. Aside from the library functions designed for that purpose, there is no guarantee of atomic read-modify-write, such as in the case of increment or decrement.

So as long as you're not concerned about running the slow path multiple times for the initial / stale value, or issues relating to register usage etc : you should be fine.

Personally I'd probably do the CacheIsStale() check in a timer (at a frequency of your choosing) that sets the field to null when it detects staleness, rather than constantly checking two conditions. You could even do the refresh inside the timer callback and set the static field to the new reference instead, so:

public string GetValue()
    => (_myStaticCache ?? FetchAndAssign()).Value1;
private MyStaticCache FetchAndAssign()
    => _myStaticCache = GetValueFromSource();
private void TimerCallback()
{
    if (_myStaticCache is null || _myStaticCache.CacheIsStale())
        FetchAndAssign();
}

Note that we can only comment on _myStaticCache; what .Value1 does is up to your code, and may or may not be thread-safe.

Upvotes: 6

Related Questions