>>I just wrote the below but you know what it's 12:17am and I'm suddenly not so sure of anything so tell you what, I'm going to look at the IL tomorrow morning when I get in the office and have had a coffee and I'll update this entry then...

A reader asked about some clarification on the “MethodImplOptions.Synchronized” option as described in Chapter 5 of the Performance and Scalability PAG.

The documentation says:

“Specifies the method can be executed by only one thread at a time.”

And the PAG indicated:

“MethodImplOptions.Synchronized enumeration option. This provides the ability to grant exclusive access to an entire method, which is rarely a good idea.”

However the article by Kit George correctly indentifies that “lock(this)” is used to implement “MethodImplOptions.Synchronized” and further describes why that isn't so useful.

The PAG wording should have been more specific as to how the interlock happens.

I think we should have been more clear that this is one of those ones that goes into the “don't do that” bucket along with the others like “lock(typeof(foo))”

Comments (8)

  1. Steve says:

    I understand the reasons for not doing lock(typeof(foo)) or lock(this)… However, why do most documents showing examples of lock show lock(this)?

    It seems to be rather inconsistent… Maybe it’s to covey the locking concept to the least common demoniator of developers?

  2. Rico Mariani says:

    Sadly I just can’t get them to stop using "lock(this)" fast enough… so many docs so little time.

  3. Pavel Lebedinsky says:

    There’s nothing inherently wrong with lock(this).

    Using lock() on instances of classes that you don’t control is what causes problems.

    May be classes that are commonly abused in this way (like colections) should use private lock objects, but even this is not obvious to me.

    Blindly replacing lock(this) with lock(this.privateLockObject) everywhere will just increase the number of allocated objects for no good reason.

  4. Rico Mariani says:

    Hmmm… how about this hypothetical argument:

    There’s nothing inherently wrong with <public data members>.

    <Modifying public data members> of classes that you don’t control is what causes problems.

    Therefore you should feel free to use <public data members>.

    Doesn’t look so good does it.

    It all depends on whether or not you think you can trust your callers to not mess with your locking semantics. And whether or not you are exposed to such a problem in the first place.

    Whether or not doing lock(this) causes an explosion of lock objects really depends on what your locking policy is. There’s lots of good guidance available on this most of which says that you shouldn’t put locks in the bulk of your objects anyway because the lock belongs fairly high on your abstraction stack where you have knowledge of which operations need to be atomic.

    Whatever you do, you should understand the risk you are taking (if any) and be happy with it.

    If the extra step makes you think twice about having locking semantics in low level classes (often, but not always, a bad idea) it might very well be a net good thing for perf as well as robustness.

    Kindly take this advice with the usual helping of salt.

  5. Rico Mariani says:

    Another interesting feature of having a seperate object for locking instead of lock(this) is that you can can then still hash the main object (or do other objhdr stuff) without having to create a syncblock. Creating the second object might actually use *less* space depending on what you did with the first object.

  6. Pavel Lebedinsky says:

    The public member analogy is a good one. Note that the syncblock is essentially a (lazily allocated) public field, and I think this is the real root of the problem.

    For most objects syncblock is not even needed, and when it’s needed it could just as well be a separate object that’s not accessible from the outside.

    I think you might actually be right. May be it’s better to always lock private objects and pretend that lock(this) or the ability to lock arbitrary objects don’t exist.

    On the other hand (since this is a performance blog after all) may be storing the lock as part of the object header is better from the locality of reference point of view.

  7. Rico Mariani says:

    If you allocate the private object that you’re going to lock in the .ctor of the object that’s doing the locking then they are almost guaranteed to be side-by-side in memory so locality shouldn’t be a problem.

    But those are famous last words 🙂

Skip to main content