C# Is it safe to use classes as storage space and access them from different threads?

Question

Say, I have a static class like this

static class PCstatus
    {
        public static class Cpu
        {
            //CPU loads
            public static int lt;
            public static int l1;
            public static int l2;
            public static int l3;
            public static int l4;
            //CPU Temp
            public static double t0;
            //Frequency
        }}

Which I'm using as a storage space(should I be doing that?)

And I have 5-6 threads that periodically change different variables in this class(Note: No two threads change the same value) i.e:

First thread:

PCstatus.lt = 0;,
thread.sleep(1000);

Second

PCstatus.l1 = 0;,
thread.sleep(1000);

And then I have another thread that periodically reads all the values from the class, parse them and send them over serial.

Is this a sane way to do it? There is no locking mechanism in the class, so theoretically, one of the threads could try to change a var while the final thread is reading it.

I'm not sure if such a thing can happen, I've run this program for days. So far, haven't noticed any strange behavior.

I can implement a locking mechanism to the class. (bool _isBeingUsed) and make the threads check that value before performing any operation, but I'm not sure if it's necessary.

I know the proper way to output values from threads is to use delegates, but if it's not really necessary, I could do without the added complexity they bring.

`double` reads and rights are not thread safe unless it's a 64-bit application running on a 64 bit processor. So there's that. — user1666620, May 23 '16 at 12:50
If you only need *latest* (most recent) values, then storage + polling is ok (take care about `double`, using `float` may be the easiest *fix*). Otherwise it may worth to create an event and pass values as *event arguments*. You can even store values in private fields and don't have any multi-threading hassle. — Sinatr, May 23 '16 at 13:23
@Sinatr I thoguht about setting up set; get; for every value, but didn't think it'd help with thread safety. Thanks for the tip. — flanker, May 23 '16 at 14:13
Possible duplicate of [Volatile vs. Interlocked vs. lock](http://stackoverflow.com/questions/154551/volatile-vs-interlocked-vs-lock) — , May 24 '16 at 06:01

score 1 · Answer 1 · answered May 23 '16 at 12:55

Reads and writes to int values in C# are atomic, so you'll never have to worry about data shearing.

However, writing to multiple values within the class is not atomic, so in your example:

First thread:

PCstatus.lt = 0;
thread.sleep(1000);

Second

PCstatus.l1 = 0;
thread.sleep(1000);

There's no guarantee that just because thread 3 sees that lt is 0 that it will also see that l1 is zero. You've potentially got data race issues here.

Also, just because a thread writes to a variable it doesn't mean that other threads will see its value immediately. Instruction reordering of instructions, compiler reordering of instructions and CPU caching strategies may conspire to prevent the write making its way back to main memory and into another thread.

If you're only ever going to change single values from a thread then use methods on the Interlocked class to ensure that your changes are visible across threads. They use a memory barrier to ensure that read/writes to variables propagate across threads.

If you're going to write multiple values in one hit, or if you want to read multiple values in one hit then you'll need to use a lock.

In my application, none of the values stored in the class are critical for the operation. Since they are all refreshed at least once a second, a few messed up vals every once in a blue moon is no big deal. As long as it won't throw an exception and crash the program. Thanks for pointing the Interlocked. — flanker, May 23 '16 at 14:16

score 0 · Accepted Answer · answered May 23 '16 at 13:05

0

No locking is required, but you should declare those fields volatile to ensure that updates from one thread can be picked up immediately by other threads.

See: https://msdn.microsoft.com/en-us/library/x13ttww7.aspx

Note that you can't declare a double to be volatile. I think for your application you could probably just use a float instead. Otherwise you can use a class that contains an immutable double value.

answered May 23 '16 at 13:05

Matt Timmermans

53,709
3
46
87

Yes. Volatile is what I was looking for. Thanks. – flanker May 23 '16 at 14:19
@flanker, read [this](http://stackoverflow.com/q/72275/1997232) before starting using `volatile` blindly. – Sinatr May 23 '16 at 14:33
@Sinatr thanks for the link. I get what it's saying, but in my case integrity of the values stored in volatile fields isn't really critical. I just don't want my program to crash while trying to read and write at the same address simultaneously. Volatile keyword seems to guarantee that. – flanker May 23 '16 at 16:16
You don't have to worry about crashing, since reads and writes of ints and floats are atomic. Without the volatile keyword, though, the write might not go all the way out to main memory in a reasonable time, and the read might get a cached value. @Sinatr, that link is a little shrill. There's lots of stuff you shouldn't use volatile for, but flanker provides enough information in the question to determine that volatile is appropriate for his use case. – Matt Timmermans May 23 '16 at 19:07

Sinatr · Answer 3 · 2016-05-23T14:34:18.473

And I have 5-6 threads that periodically change different variables in this class

Instead of having single storage for results of 5-6 workers you can supply each worker with event. Then anyone who need results can subscribe to it and create local storage, means no thread issues anymore.

Something like

public static class CPUStats
{
    public static EventHandler<CPUEventArgs> Measured;

    public static CPUStats()
    {
        Task.Factory.StartNew(() =>
        {
            while(true)
            {
                ... // poll CPU data periodically
                Measured?.Invoke(null, new CPUEventArgs() { LT = lt, L1 = l1, ... });
            }
        }, TaskCreationOptions.LongRunning);
    }
}

public static class StatsWriter
{
    static int lt;
    static int l1;
    ...

    public static StatsWriter()
    {
        CPUStats.Measured += (s, e) =>
        {
           lt = e.LT;
           l1 = e.L1;
        }
    }

    public static void Save()
    {
        var text = $"{DateTime.Now} CPU[{lt},{l1}...]";
        ... // save text
    }
}

Thought about that but; I have around 20 threads in total that changes 60+ valuables. Creating events for them all just seemed like a chore. That is the proper way of doing what I want, but in my case it seems like an unnecessary chore. — flanker, May 23 '16 at 16:10

C# Is it safe to use classes as storage space and access them from different threads?

3 Answers3