V3090. Unsafe locking on an object.

The analyzer detected a code fragment with unsafe locking on an object.

This diagnostic is triggered in the following situations:

• locking on 'this';
• locking on instances of classes 'Type', 'MemberInfo', 'ParameterInfo', 'String', 'Thread';
• locking on a public member of the current class;
• locking on an object that resulted from boxing;
• locking on newly created objects.

The first three scenarios may cause a deadlock, while the last two ones caus thread synchronization to fail. The common problem of the first three scenarios is that the object being locked on is public: it can be locked on elsewhere and the programmer will never know about that after locking on it for the first time. As a result, a deadlock may occur.

Locking on 'this' is unsafe when the class is not private: an object can be locked on in any part of the program after creating its instance.

For the same reason, it is unsafe to lock on public class members.

To avoid these issues, you just need to lock on, for example, a private class field instead.

Here is an example of unsafe code where the 'lock' statement is used to lock on 'this':

class A
{
  void Foo()
  {
    lock(this)
    {
      // do smt
    }
  }
}

To avoid possible deadlocks, we should lock on, for example, a private field instead:

class A
{
  private Object locker = new Object();
  void Foo()
  {
    lock(locker)
    {
      // do smt
    }
  }
}

Locking on instances of classes 'Type', 'MemberInfo', and 'ParameterInfo' is a bit more dangerous, as a deadlock is more likely to occur. Using the 'typeof' operator or methods 'GetType', 'GetMember', etc. on different instances of one type will produce the same result: getting the same instance of the class.

Objects of types 'String' and 'Thread' need to be discussed separately.

Objects of these types can be accessed from anywhere in the program, even from a different application domain, which makes a deadlock even more likely. To avoid this issue, do not lock on instances of these types.

Let's see how a deadlock occurs. Suppose we have an application (Sample.exe) with the following code:

static void Main(string[] args)
{
  var thread = new Thread(() => Process());
  thread.Start();
  thread.Join();
}
static void Process()
{
  String locker = "my locker";
  lock (locker)
  { 
    ....
  }
}

There is also another application with the following code:

String locker = "my locker";
lock (locker)
{
  AppDomain domain = AppDomain.CreateDomain("test");
  domain.ExecuteAssembly(@"C:\Sample.exe");
}

Executing this code will result in a deadlock, as it uses an instance of the 'String' type as a locking object.

We create a new domain within the same process and attempt to execute an assembly from another file (Sample.exe) in that domain, which results in both 'lock' statements locking on the same string literal. String literals get interned, so we will get two references to the same object. As a result, both 'lock' statements lock on the same object, causing a deadlock.

This error could occur within one domain as well.

A similar problem is observed when working with the 'Thread' type, an instance of which can be easily created by using the 'Thread.CurrentThread' property, for example.

To avoid this issue, do not lock on objects of types 'Thread' and 'String'.

Locking on an object of a value type prevents threads from being synchronized. Note that the 'lock' statement does not allow setting a lock on objects of value types, but it cannot protect the 'Monitor' class with its methods 'Enter' and 'TryEnter' from being locked on.

The methods 'Enter' and 'TryEnter' expect an object of type 'Object' as an argument, so if an object of a value type is passed, it will be 'boxed', which means that a new object will be created and locked on every time; therefore, the lock will be set (and released) on these new objects. As a result, thread synchronization will fail.

Consider the following example:

sealed class A
{
  private Int32 m_locker = 10;

  void Foo()
  {
    Monitor.Enter(m_locker);
    // Do smt...
    Monitor.Exit(m_locker);
  }
}

The programmer wanted to set a lock on the private field 'm_locker', but it will be actually set (and released) on the newly created objects resulting from 'boxing' of the original object.

To fix this error, we just need to change the type of the 'm_locker' field to a valid reference type, for example, 'Object'. In that case, the fixed code would look like this:

sealed class A
{
  private Object m_locker = new Object();

  void Foo()
  {
    Monitor.Enter(m_locker);
    // Do smt...
    Monitor.Exit(m_locker);
  }
}

A similar error will appear when the 'lock' construction is used, if there will be packing of an object in the result of casting:

Int32 val = 10;
lock ((Object)val)
{ .... }

In this code there will be locking on objects, obtained in the result of boxing. There will be no thread synchronization, because new objects will be created after the boxing.

Locking on the newly created objects will be erroneous. An example of such code may be as follows:

lock (new Object())
{ .... }

or as this:

lock (obj = new Object())
{ .... }

Locking will be also done on different objects, because new objects are created every time the code is executed. Therefore, the threads will not be synchronized.

This diagnostic is classified as: