thread safe static class

Question

I would like to know if the following static class would be thread safe or not and why?

public static class Settings
{
    public static string AStringSetting { get; set; }
    public static int AIntSetting { get; set; }

    public static void Load()
    {
        // load values from database
    }
}

I have a multithread windows service that loads the setting a the start of the service. I would like to add some kind of auto refresh in my service. I made some test running 200+ threads reading and writing the public properties and I was not able to make the process crash...

Is that class thread safe? Why?

You can't have non static properties in a static class. Did you try running this? — Candide, Jan 25 '15 at 11:33
"not crashing" is not the same as thread-safe. You will have to think about what kind/level of thread-safety you're after, the question isn't clear. — H H, Jan 25 '15 at 11:33
@Candide I wrote the code sample from my Phone on the train on my way to the office. — Baral, Jan 25 '15 at 11:37
@Henk I need the service to run in High Availability mode, this is why I need to have the settings to auto refresh. I cannot afford to restart the service during business hours — Baral, Jan 25 '15 at 11:39
@Baral Look at SQL Depdency (with Service Broker) It can be very useful to push messages to the service so that the settings are refreshed. Actually, each running thread can subscribe to the queue, and receive it's own refresh messages. Static properties are not thread safe, you have to use locks for reading and writing to properties. — Candide, Jan 25 '15 at 11:43

score 0 · Answer 1 · answered Jan 25 '15 at 11:36

0

Just because you couldn't make your program crash doesn't mean that your class is thread safe. Check http://en.wikipedia.org/wiki/Thread_safety. In your case race conditions can occur, because access to your properties is not synchronized.

answered Jan 25 '15 at 11:36

smiech

725
4
8

No, you won't have race-conditions in these simple properties. Plenty of other things could go wrong though. – H H Jan 25 '15 at 11:40
Why is that? If those 2 props are parts of a complete setting, you can read half-modified state in one of the threads. – smiech Jan 25 '15 at 11:43
@HenkHolterman, can you give me some example please? This is why I posted this question because I was not confident about this but I would appreciate to have a better understanding – Baral Jan 25 '15 at 11:43
@smiech - Yes, if the settings as a whole must be coherent then you're right. That's not obvious though and not specified here. – H H Jan 25 '15 at 11:50
@Baral - there is no general answer, we'd have to know how those settings are used. – H H Jan 25 '15 at 11:51

Dirk Trilsbeek · Answer 2 · 2015-01-26T07:57:07.410

I'm no expert on threading but afaik thread safety is not about crash-prevention. It's more about reliable results. In your code example your properties AStringSetting and AIntSetting are never used, so it's not possible to determine if there may be a threading issue. Consider this code:

if (AStringSetting.Equals("TEST"))
{
    // AStringSetting may not be "TEST" anymore, although it was until a moment ago. 

    if (AIntSetting < 10)
    {
       // no idea what AIntSetting contains now. It was below 10 just a moment ago, though.
       AIntSetting = AIntSetting + 1;
    }
}

this code would not be thread safe. For instance, another thread could modify one of the two properties while the first line is executed. Or worse, after the first line has been executed. Depending on what that code would do, you could end up with inconsistent results. You would not be able to rely on the fact that you just tested the contents of AStringSetting and AIntSetting, because you never know if another thread didn't just change them.

To make your class thread safe, you should implement a thread locking mechanism and let all methods use that mechanism (in your case that could be a private static Object and the lock command) to block access to the static properties from other threads. Your getters and setters should also use that locking mechanism to make sure properties are neither read nor changed from different threads.

 private static string mStringSetting;
 public static string AStringSetting { 
     get{
         lock(mThreadLock) { return mStringSetting; }
     } 
     set {
         lock(mThreadLock) { mStringSetting = value; }
     }
 }

 private static int mIntSetting;
 public static int AIntSetting { 
     get{
         lock(mThreadLock) { return mIntSetting; }
     } 
     set {
         lock(mThreadLock) { mIntSetting = value; }
     }
 }
 private static Object mThreadLock = new Object();

 public static void IncreaseIntIfLowerThan10AndStringIsTest()
 {
     lock(mThreadLock)
     {
         if (AStringSetting.Equals("TEST"))
         {
             // AStringSettig is still "TEST", no other thread can change it due to the lock
             if (AIntSetting < 10)
             {
                 AIntSetting = AIntSetting + 1;
             }
         }
     }
 }

And even that may not be enough if the properties have some kind of logic connection and depend on another. If that is the case, you may have to implement thread safe methods implementing your business logic in one, thread-safe step and block at least write-access to the properties from outside the class altogether.

score -1 · Answer 3 · edited May 23 '17 at 11:43

-1

You might want to look at the answer here: https://stackoverflow.com/a/435045/896697. Jon Skeet makes a really interesting point that in certain cases the JIT compiler can cache values. And since your point is to get the latest ( or current ) version of that application setting, you might not reach your goal ( regardless of thread-safety ).

edited May 23 '17 at 11:43

Community

1
1

answered Jan 25 '15 at 12:01

Jochen van Wylick

5,303
4
42
64

thread safe static class

3 Answers3