Do not lock on objects with weak identity

Question

How can i resolve this reliability issue(Do not lock on objects with weak identity)?
Should i lock it with "object locker = new object();"?

lock (typeof(ObjectCultures))
 {
     if (!requestCache.Contains(GetCacheKey(objectId, cultureId)))
         {
             requestCache.Add(GetCacheKey(objectId, cultureId), responseStr);
         }
 }

Yes, use `object locker = new object()` and lock on that (just be sure locked variable is a shared class member, not a local variable otherwise lock is useless). — Adriano Repetti, Sep 16 '14 at 11:17
Locking on a type is considered bad practice. Using a lock object is much better. — Lucas Trzesniewski, Sep 16 '14 at 11:17

Daniel Kelley · Answer 1 · 2014-09-16T11:23:54.833

Create a new static instance of an object and lock on that:

private static readonly object locker = new object();

lock (locker)
{
    ....
}

This is on the assumption you are locking inside a static method (due to your use of typeof(T) instead of the this which is also bad practice).

Obviously if you are inside an instance method remove the static from the declaration.

This article by Jon Skeet will help explain why the issue is being flagged in the first place.

score 0 · Accepted Answer · answered Sep 16 '14 at 11:17

0

You should lock an object not a type. Declare and instance this object in a main scope of your class, for sample:

public class SomeClass
{
    private static readonly object _lock = new object();

    public void SomeMethod()
    {
       lock (_lock)
       {
           // some code
       }
    }
}

This code will block all thread that are trying to execute this block and make them wait for finish and then execute the next thread.

answered Sep 16 '14 at 11:17

Felipe Oriani

37,948
19
131
194

1

Assuming there are no static resources used within the method, he shouldn't be using a static synchronization object. – Servy Sep 16 '14 at 15:43

score 0 · Answer 3 · answered Sep 16 '14 at 11:19

Effectively, you need to put the lock in a static object reference. An object typed reference should be enough.

public class X
{
     private readonly static _syncLock = new object();

     public void DoStuff()
     {
          lock(_syncLock) 
          {
             // Critical section
          }
     }
}

score 0 · Answer 4 · answered Sep 16 '14 at 11:31

Exactly how you should lock.. depends on the context, and the bigger picture of code you are trying to write..

What you do want to avoid, is locking on objects of type indicated in the link you already got - http://msdn.microsoft.com/en-us/library/ms182290.aspx

Do note that the handful of types listed here is not the full list...

• MarshalByRefObject

• ExecutionEngineException

• OutOfMemoryException

• StackOverflowException

• String

• MemberInfo

• ParameterInfo

• Thread

Full list would include instance of any type which is derived directly or indirectly from any of the above mentioned types.. Note that System.Type extends System.Reflection.MemberInfo http://msdn.microsoft.com/en-us/library/system.type.aspx And hence the warning, when using lock (typeof(ObjectCultures))

Do not lock on objects with weak identity

4 Answers4