CODEWITHSUNDEEP
c#.netmultithreadingatomic
VSZM
VSZM

Reputation: 1420

C# thread safe static member

I have a C# class with a static member, that is read from multiple threads and written in one thread.

As far as I know Uint64 read and write is not an atomic operation on all systems, so I have to manually guarantee thread safety.

I had a few ideas about how to do this.

  1. Do it with and atomic wrapper class, like std::atomic in c++. Is there something similar implemented in C#?

  2. Use the volatile modifier with static field. However this is not allowed. Why?

  3. I finally did the following: 

    private static object tick_time_lock;
private static UInt64 _currentTickTime;
public static UInt64 CurrentTickTime 
{
    get
    {
        return _currentTickTime;
    }
    set
    {
        lock (tick_time_lock)
        {
            _currentTickTime = value;
        }
    }
}

Is this the correct way of making this field thread-safe?

Upvotes: 5

Views: 2430

Answers (5)

Euphoric
Euphoric

Reputation: 12849

No, you are doing it completely wrong. Having lock on only read/write operations is not going to help you. For example, this code is not thread safe, even though both get and set might be guarded by locks:

var time = Clock.CurrentTickTime;
time += 1
Clock.CurrentTickTime = time

You would need to put lock all around this code to make it thread-safe.

Also, making all components of a system thread-safe is not going to guarantee the whole system is thread-safe. It will actually increase posibilities of deadlocks and makes debugging harder.

Simply said, you are approaching thread safety from completely wrong angle. First, forget about synchronizing global state. It is extremely hard to have both global state and thread safety. Instead, have all state thread-local and only synchronize this state at precisely defined points, where you can guarantee a thread safety.

Upvotes: 1

Mike Strobel
Mike Strobel

Reputation: 25623

Is this the correct way of making this field thread-safe?

A monitor lock is meaningless unless all accesses of a given resource are synchronized. Putting a lock around the set accessor is rather useless unless you also lock on the get accessor. As you say, reads and writes of UInt64 values are not atomic on all platforms. What happens if the field is read in the get accessor when only the first word has been written in the set accessor? You'd get a torn read.

Use the volatile modifier with static field. However this is not allowed. Why?

The C# language designers felt it was beneficial to guarantee that all volatile field accesses are atomic. As a trade-off, you cannot declare any 64-bit field as volatile. I do not know for certain why this decision was made. Perhaps they wanted to avoid adding "hidden" overhead to some volatile read/write operations and instead require developers to depend on framework-level facilities like Thread.Volatile[Read/Write](ref long) for handling 64-bit values.

Do it with and atomic wrapper class, like std::atomic in c++. Is there something similar implemented in C#?

Yes. There are framework-level atomic operations exposed through the System.Threading.Interlocked class, including Read, Exchange, CompareExchange.

Upvotes: 6

Will Dean
Will Dean

Reputation: 39500

For 64-bit integers, you might also consider using members of the Interlocked class in the .NET framework:

http://msdn.microsoft.com/en-us/library/system.threading.interlocked(v=vs.110).aspx

Upvotes: 2

Kirill Polishchuk
Kirill Polishchuk

Reputation: 56162

Look at Interlocked class, it provides atomic operations for variables that are shared by multiple threads.

Upvotes: 5

Szymon
Szymon

Reputation: 43023

I think you will need to instantiate your lock object. Also, use the lock for get too.

private static Object tick_time_lock = new Object();
private static UInt64 _currentTickTime;
public static UInt64 CurrentTickTime 
{
    get
    {
        lock (tick_time_lock)
        {
            return _currentTickTime;
        }
    }
    set
    {
        lock (tick_time_lock)
        {
            _currentTickTime = value;
        }
    }
}

Upvotes: 0

Related Questions