Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

C# lock and code analysis warning CA2002

In my application I have a form that starts synchronization process and for number of reasons I want to allow only one synchronization to run at a time. So I've added a static bool field to my form indicating whether sync is in progress and added a lock to set this field to true if it wasn't already set so that first thread could start synchronization but when it's running every other thread that will try to start it will terminate.

My code is something like this:

internal partial class SynchronizationForm : Form
{
    private static volatile bool workInProgress;

    private void SynchronizationForm_Shown(object sender, EventArgs e)
    {
        lock (typeof(SynchronizationForm))
        {
            if (!workInProgress)
            {
                workInProgress = true;
            }
            else
            {
                this.Close();
            }
        }
    }
}

This is working well but when I run Code Analysis on my project I'm getting the following warning message:

CA2002 : Microsoft.Reliability : 'SynchronizationForm.SynchronizationForm_Shown(object, EventArgs)' locks on a reference of type 'Type'. Replace this with a lock against an object with strong-identity.

Can anyone explain to me what's wrong with my code and how can I improve it to make the warning gone. What does it mean that object has a strong-identity?

like image 425
RaYell Avatar asked Oct 23 '09 13:10

RaYell


2 Answers

What is wrong is that you are locking on something public (typeof(SynchronizationForm)) which is accessible everywhere from your code and if some other thread locks on this same thing you get a deadlock. In general it is a good idea to lock only on private static objects:

private static object _syncRoot = new object();
...
lock (_syncRoot) 
{

}

This guarantees you that it's only SynchronizationForm that could possess the lock.

like image 170
Darin Dimitrov Avatar answered Oct 30 '22 13:10

Darin Dimitrov


From the MSDN explanation of the rule

An object is said to have a weak identity when it can be directly accessed across application domain boundaries. A thread that tries to acquire a lock on an object that has a weak identity can be blocked by a second thread in a different application domain that has a lock on the same object.

Since you can't necessarily predict what locks another AppDomain might take, and since such locks might need to be marshalled and would then be expensive, this rule makes sense to me.

like image 43
Doug McClean Avatar answered Oct 30 '22 12:10

Doug McClean