Should my lock object be static when used to update static properties

Question

I have a non-static class with a static property. The static property is a dictionary, which is built when it is first needed.

So the code that uses the dictionary looks something like this:

// Ensure dictionary is built
if (MyDictionary == null)
{
    lock (LockObject)
    {
        if (MyDictionary == null)
        {
            MyDictionary = new();
            BuildDictionary(MyDictionary);
        }
    }
}

What I'm not clear on is if LockObject should be static.

I'm leaning to it not being static but some information I found online seems to indicate it should be.

If the lock object is static, wouldn't that mean other instances of the same class would not be blocked?

Lock object should be static also. Otherwise each instance will use its own lock object and therefore won't block other instances. By the way, why not use the [`Lazy`](https://learn.microsoft.com/en-us/dotnet/api/system.lazy-1)? — Dmitry, Sep 17 '21 at 22:56
@Dimitry: I was thinking other code is blocked as long as the lock object is a different instance. I wish I could find a good article on this. Microsoft's documention seemed lacking. I can look at Lazy<> but, here, `BuildDictionary()` is actually an abstract method that is overridden in derived classes and I'm not sure `Lazy<>` wouldn't complicate things. — Jonathan Wood, Sep 17 '21 at 23:08
This is just a +1 for @dimitry's suggestion for `Lazy`. There's lots of ways to do singleton initialization wrong; using Lazy means you don't have to think about them. I find it simplifies my code. You probably still want a static lock on access to the dictionary though. — Flydog57, Sep 17 '21 at 23:36
If you do do it this way (and you shouldn't, you should use `Lazy` or `LazyWithNoExceptionCaching`) then assign to `MyDictionary` **after** populating a temporary dictionary not _first_. The code, as is, will sometimes result in some callers seeing an empty dictionary (or even worse, getting exceptions since they are reading from a dictionary as another thread is writing to it). — mjwills, Sep 17 '21 at 23:45
`If the lock object is static, wouldn't that mean other instances of the same class would not be blocked?` Why do you think that? Think of locks like this - it is a talking stick. You can't talk without a talking stick. If you want only one person talking, how many sticks do you want? 1, or 1 per person? Clearly just 1. Hence - `static` is what you need. — mjwills, Sep 17 '21 at 23:46
`I was thinking other code is blocked as long as the lock object is a different instance` Your logic is the wrong way around. — mjwills, Sep 17 '21 at 23:49
@mjwills: Trying to ask how something works doesn't involve logic, wrong or right way around. It's just my understanding, which I've already acknowledged is lacking. I even asked for good articles that cover this aspect of it, which I'm having trouble finding. Telling me my logic is wrong is misunderstanding what I'm saying. — Jonathan Wood, Sep 18 '21 at 01:18
It seemed to me, based on your comment, that you have a mental model of how locks work. I was pointing out your mental model was flawed / wrong / not correct. Apologies if I offended you in any way. Either way - your current code is wrong (i.e. not thread-safe) regardless - so better to use `Lazy` or `LazyWithNoExceptionCaching`. — mjwills, Sep 18 '21 at 02:00
[Double-checked locking](https://en.wikipedia.org/wiki/Double-checked_locking) *"The pattern, when implemented in some language/hardware combinations, can be unsafe. At times, it can be considered an anti-pattern."* — Theodor Zoulias, Sep 18 '21 at 06:28
@TheodorZoulias: The same articles seems to indicate it can be used efficiently in C#, and in fact suggests the `Lazy<>` class does so internally. — Jonathan Wood, Sep 18 '21 at 18:22
Jonathan yeap, you are right. The article contradicts the message that I was trying to propagate. :-) Even worse, the [source code](https://referencesource.microsoft.com/mscorlib/system/Lazy.cs.html) reveals that the `Lazy` class makes use of this pattern indeed. One peculiarity is that the created value is stored as a field of an internal `Boxed` class, and not directly as a field of the `Lazy` class. I don't know how important this is, regarding the guarantee that all threads will see a fully initialized `T` value. — Theodor Zoulias, Sep 18 '21 at 19:39
I don't like this pattern honestly, mainly because I've used it myself extensively at a point of my life when I was writing multithreaded code without knowing what I was doing. After studying multithreading systematically for two years, I now know that writing low lock code requires to be [very smart](https://stackoverflow.com/a/2529773), and also [very knowledgable](https://stackoverflow.com/a/66490395) about cache coherency protocols and stuff. I am neither if these, so I'll stick with my `lock` thank you. :-) — Theodor Zoulias, Sep 18 '21 at 19:42

Rowan Smith · Answer 1 · 2021-09-18T01:26:54.333

What I'm not clear on is if LockObject should be static?

There's no right or wrong answer to this particular question, it depends on what you want to do.

Generally,

If the object you are protecting is a static then use a static to lock it.

And, for objects in Instances of the class that need to be locked, don't use static.

eg:

class Thing
{
    /// Global objects
    static object _lockPublicDictionary;
    static SortedDictionary<int, string> PublicDictionary;

    /// Per instantiation objects
    object _lockPrivateDictionary;
    SortedDictionary<int, string> PrivateDictionary;
}

If for some reason you want to globally lock an object of a class for all instances of that class then you could use a static lock.

static object _masterLock
SpecialObject Special;

I can't think of a use case for this off the top of my head, maybe for some kind of syncronised object, where all the instances know about each other and one instance can communicate with the others via some back-channel.

Also, I suggest having a look at the Singleton Pattern, which results in only one instance ever of a class. It's similar to a static class and based on your questions, could be what you are actually looking for.

Strictly speaking out of 3 answers to "what [lock granularity](https://en.wikipedia.org/wiki/Lock_%28computer_science%29#Granularity) to use" - more locks than data pieces", "one lock per data piece", "less locks than data pieces" there is definitely one wrong answer - more locks than data (which is what OP suggested as an option)... And here is a use case for single lock object - it is impossible to get into deadlock when you have only one lock object which may be more important than some potential lock contention. — Alexei Levenkov, Sep 18 '21 at 00:16
"There's no right or wrong answer to this" - Yes, there is. The same `lock` object must be used by all code trying to enter the region being locked if they are accessing the same data. — Enigmativity, Sep 18 '21 at 01:16
@Enigmativity of course. But that's not what the op asked :-) — Rowan Smith, Sep 18 '21 at 01:28

Should my lock object be static when used to update static properties

1 Answers1