Thread safety consideration for static property as an Instance of that class

Question

Please consider this code:

public class BusinessClass
{
    static BusinessClass myClass { get; set; }
    Repository repo;

    public BusinessClass()
    {
        if (repo == null)
            repo = new RepositoryClass();
    }

    public static BusinessClass Instance
    {
        get
        {
            if (myClass == null)
                myClass = new BusinessClass();
            return myClass ;
        }
    }

    public void Update(Entity Item)
    {
        repo.Update(Item);
    }
}

and I want to use this BL in my web page like this:

BusinessClass.Instance.Update(Item);

My question is: is this code problematic for thread safety? Can two people come together at the same time in Update method?

Thanks

Two requests will be handled by two different calls to the same method, each having their own stack so local variables will not be shared. But, `BusinessClass.Instance` will be shared. Whether that is OK or not is something you will have to decide yourself. — Lasse V. Karlsen, Dec 18 '18 at 13:20

score 5 · Accepted Answer · answered Dec 24 '18 at 09:15

First case - asp.net session lock

If you use asp.net forms and the session of asp.net, then the session is lock the entire call for all users, so you don't need to extra take care to synchronize that.

Relative questions :

Does ASP.NET Web Forms prevent a double click submission?
Trying to make Web Method Asynchronous
Web app blocked while processing another web app on sharing same session
What perfmon counters are useful for identifying ASP.NET bottlenecks?
Replacing ASP.Net's session entirely

Second case - the local thread.

If you not use the asp.net session, or if you open extra threads on the same call, then you need to lock the manipulation of static data.

public class BusinessClass
{
    private static readonly object oLock = new object();

    static BusinessClass myClass { get; set; } = null;
    Repository repo;

    public BusinessClass()
    {
        if (repo == null)
            repo = new RepositoryClass();
    }

    public static BusinessClass Instance
    {
        get
        {
            if myClass == null)
            {
                lock (oLock)
                {
                    if myClass == null)
                        myClass = new BusinessClass();
                }
            }
            return myClass  
        }
    }

    public void Update(Entity Item)
    {
        repo.Update(Item);
    }
}

Final case - the global change

If you wish to double check a global change on the database or on the files, or on anything that can change from simulate edit of the system - on a web platform that runs on web garden (multi pools for the same site)... and ignore the session...

Then you need mutex to synchronize all calls.

By using mutex, you also need to check just before the update, that the record is still the same and if not you signaling that some one else change it and the current user will overwrite it.

More to consider - multi-user environment.

Think this scenario.
User A and B, load the same page, the same data, and each of one change them.

the users are going to save each data - not the same moment - but with a lot of time different. The data that you write down is the last one saved. One of the user will lost their changes.

So you need some more to consider and made a lot more synchronization signaling in a multi user environment if your users update the same data - beyond the locking system.

Thanks dear @Aristos. I'm using ASP.Net web forms and is my code thread-safe? — Arian, Dec 26 '18 at 13:43
Can you explain more about first case? Does it work on static class and static properties? — Arian, Dec 26 '18 at 14:54
@Arian The session that asp.net is use, is a thread safe module - that locks each entire call from user - I have include some links that explain that case. Actually is hard to avoid this entire lock. For started site is a lot of help because is make a good synchronization of your edits. Now, its not bad to also use `lock(){}` on your static manipulation of your variables just in case. — Aristos, Dec 26 '18 at 19:15
@Arian Its work with anything, actually is lock every call - you can try some experiments your self - add some thread delay on your page, and try to call it many times with open session, then try it again with session off - and you see the results. At the end you see that is harder to avoid the synchronization than anything else - ending to remove entire the session and make a new session from scratch if you try to have multi-users work at the same time do long time jobs. That's why asp.net handlers did not have session by default... to been able to download large data with out lock. — Aristos, Dec 26 '18 at 19:17
It is even worse - consider web far mscnarios having multiple copies of the static instance because tehy run in a redundant fashion or on multiple application pools. — TomTom, Jan 15 '19 at 06:58
@TomTom So this static instance must be some how synchronize - I use a file that hold that synchronization. Other static data dont actual affect the results - for example they are they same and not change - — Aristos, Jan 15 '19 at 07:31

score 0 · Answer 2 · answered Dec 18 '18 at 15:26

It's not thread-safe. Here is just one example of what could happen:

Thread1: if (myClass == null) <- receives true because it's null

Thread1: [temp1] = new BusinessClass(); <-- [temp1] exists implicitly in Thread 1

Thread2: if (myClass == null) <- receives true because it's still null

Thread2: [temp2] = new BusinessClass(); <-- [temp2] exists implicitly in Thread 2

Thread2: myClass = [temp2];

Thread2: return myClass; <-- returns second instance created

Thread1: myClass = [temp1];

Thread1: return myClass; <-- returns first instance created

Now each thread holds a different instance of the "singleton" Instance.

An easy-to-use thread-safe direct alternative would be to use a Lazy<BusinessClass> to hold your singleton instance instead of myClass.

Even if you are lucky, and the order is correct, you still might need a memory barrier to get the same value on both threads. More info: http://www.albahari.com/threading/part4.aspx — Denxorz, Dec 18 '18 at 15:34

Thread safety consideration for static property as an Instance of that class

2 Answers2

First case - asp.net session lock

Second case - the local thread.

Final case - the global change

More to consider - multi-user environment.