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?
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.
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.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With