Gendarme's concurrency rules are located in the Gendarme.Rules.Concurrency.dll assembly. Latest sources are available from anonymous SVN (http://anonsvn.mono-project.com/viewcvs/trunk/mono-tools/gendarme/rules/Gendarme.Rules.Concurrency/).

1.1 DoNotLockOnThisOrTypesRule
1.2 DoNotLockOnWeakIdentityObjectsRule
1.3 DoNotUseLockedRegionOutsideMethodRule
1.4 DoNotUseMethodImplOptionsSynchronizedRule
1.5 DoubleCheckLockingRule
1.6 NonConstantStaticFieldsShouldNotBeVisibleRule
1.7 ProtectCallToEventDelegatesRule
1.8 ReviewLockUsedOnlyForOperationsOnVariablesRule
1.9 WriteStaticFieldFromInstanceMethodRule

Rules

DoNotLockOnThisOrTypesRule

This rule checks if you're use lock on the current instance (this) or on a Type. Doing so means potential concurrency troubles. If you are locking this anyone else, outside your code/control, could be using a lock on your instance causing a deadlock. Locking on types is also bad since there is only one instance of each Type. Again anyone else, outside your code/control, could be locking on it. The best locking is to create your own, private, instance System.Object and lock on it.

Bad example (this):

public void MethodLockingOnThis ()
{
    lock (this) {
        producer++;
    }
}

Bad example (type):

public void MethodLockingOnType ()
{
    lock (this.GetType ()) {
        producer++;
    }
}

Good example:

class ClassWithALocker {
    object locker = new object ();
    int producer = 0;
    
    public void MethodLockingLocker ()
    {
        lock (locker) {
            producer++;
        }
    }
}

DoNotLockOnWeakIdentityObjectsRule

This rule ensures there are no locks on objects with weak identity. An object with weak identity means that it can be accessed across different application domains and may cause deadlocks or other concurrency issues. The following types have a weak identities:

System.MarshalByRefObject
System.OutOfMemoryException
System.Reflection.MemberInfo
System.Reflection.ParameterInfo
System.ExecutionEngineException
System.StackOverflowException
System.String
System.Threading.Thread

Bad example:

public void WeakIdLocked ()
{
    lock ("CustomString") {
        // ...
    }
}

Good example:

public void WeakIdNotLocked ()
{
    Phone phone = new Phone ();
    lock (phone) {
        // ...
    }
}

DoNotUseLockedRegionOutsideMethodRule

This rule ensures the method atomicity. You should put the Monitor.Enter and Monitor.Exit call in the same method, otherwise may have several headaches related to concurrency issues.

Bad example:

class BadExample {
    int producer = 0;
    
    public void EnteringMethod ()
    {
        Monitor.Enter ();
        producer++;
    }
    public void ExitingMethod ()
    {
        Monitor.Exit ();
    }
}

Good example:

class GoodExample {
    int producer = 0;
    
    public void AddProducer ()
    {
        Monitor.Enter ();
        producer++;
        Monitor.Exit ();
    }
}

DoNotUseMethodImplOptionsSynchronizedRule

This rule checks if some methods are decorated with [MethodImpl(MethodImplOptions.Synchronized)]. The runtime synchronize those methods automatically using a lock(this) for instance methods or a lock(typeof(X)) for static methods. This can cause deadlocks if misused inside your own code or if the method is visible outside the assembly since anyone else, outside your control, could be using a lock on an instance or the type and cause a deadlock. The best locking is to create your own, private, instance System.Object and lock on it.

Bad example:

[MethodImpl (MethodImplOptions.Synchronized)]
public void SychronizedMethod ()
{
    producer++;
}

Good example:

public class ClassWithALocker {
    object locker = new object ();
    int producer = 0;
    
    public void MethodLockingLocker ()
    {
        lock (locker) {
            producer++;
        }
    }
}

DoubleCheckLockingRule

This rule is used to check for the double-check pattern, often used when implementing the singleton pattern (1), and warns of its potential incorrect usage. The original CLR (1.x) could not guarantee that a double-check would work correctly in multithreaded applications. However the technique do work on the x86 architecture, the most common architecture, so the problem is seldom seen (e.g. IA64). The CLR 2 and later introduce a strong memory model (2) where a double check for a lock is correct (as long as you assign to a volatile variable). This rule wont report any defect for assemblies targetting the 2.0 (and later) runtime.

1. Implementing Singleton in C# : http://msdn.microsoft.com/en-us/library/ms998558.aspx
2. Understand the Impact of Low-Lock Techniques in Multithreaded Apps : http://msdn.microsoft.com/en-ca/magazine/cc163715.aspx#S5

Bad example:

public class Singleton {
    private static Singleton instance;
    private static object syncRoot = new object ();
    
    public static Singleton Instance {
        get {
            if (instance == null) {
                lock (syncRoot) {
                    if (instance == null) {
                        instance = new Singleton ();
                    }
                }
            }
            return instance;
        }
    }
}

Good example (for 1.x code avoid using double check):

public class Singleton {
    private static Singleton instance;
    private static object syncRoot = new object ();
    
    public static Singleton Instance {
        get {
            // do not check instance before the lock
            // this will works on all CLR but will affect
            // performance since the lock is always acquired
            lock (syncRoot) {
                if (instance == null) {
                    instance = new Singleton ();
                }
            }
            return instance;
        }
    }
}

Good example (for 2.x and later):

public class Singleton {
    // by using 'volatile' the double check will work under CLR 2.x
    private static volatile Singleton instance;
    private static object syncRoot = new object ();
    
    public static Singleton Instance {
        get {
            if (instance == null) {
                lock (syncRoot) {
                    if (instance == null) {
                        instance = new Singleton ();
                    }
                }
            }
            return instance;
        }
    }
}

NonConstantStaticFieldsShouldNotBeVisibleRule

This rule warns if a non constant public static field is detected. In a multi-threaded environment access to those fields needs to be synchronized.

Bad example:

class HasPublicStaticField {
    public static ComplexObject Field;
}

Good example:

class FieldIsReadonly {
    public readonly static ComplexObject Field = new ComplexObject();
}

class UseThreadStatic {
    [ThreadStatic]
    public static ComplexObject Field;
    
    public static InitializeThread ()
    {
        if (Field == null)
        Field = new ComplexObject ();
    }
}

ProtectCallToEventDelegatesRule

This rule checks if the call to an event delegate is safely implemented. This means the event field is verified not to be null before its use and that the field is not used directly (for the check and call) since this introduce the possible race condition.

Bad example (no check):

public event EventHandler Loading;
 
protected void OnLoading (EventArgs e)
{
    // Loading field could be null, throwing a NullReferenceException
    Loading (this, e);
}

Bad example (race condition):

public event EventHandler Loading;
 
protected void OnLoading (EventArgs e)
{
    // Loading could be non-null here
    if (Loading != null) {
        // but be null once we get here :(
        Loading (this, e);
    }
}

Good example:

public event EventHandler Loading;
protected void OnLoading (EventArgs e)
{
    EventHandler handler = Loading;
    // handler is either null or non-null
    if (handler != null) {
        // and won't change (i.e. safe from a NullReferenceException)
        handler (this, e);
        // however it is still possible, like the original code, that
        // the Loading method will be removed before, or during its
        // execution. Your code should be safe against such occurance.
    }
}

ReviewLockUsedOnlyForOperationsOnVariablesRule

This rule checks if a lock is used only to perform operations on locals or fields. If the only purpose of that critical section is to make sure the variables are modified atomatically, rather use the methods provided by System.Threading.Interlocked class to achieve better throughput.

Bad example:

lock (_lockObject) {
    _counter++;
}

Good example:

Interlocked.Increment(_counter);

Bad example:

lock (_lockObject) {
    _someSharedObject = anotherObject;
}

Good example:

Interlocked.Exchange(_someSharedObject, anotherObject);

WriteStaticFieldFromInstanceMethodRule

This rule is used to check for instance methods that writes values in static fields. This may cause problem when multiple instance of the type exists and when used in multithreaded applications.

Bad example:

static int default_value;
 
public int Value {
    get {
        if (default_value == 0) {
            default_value = -1;
        }
        return (value > default_value) ? value : 0;
    }
}

Good example:

static int default_value = -1;
 
public int Value {
    get {
        return (value > default_value) ? value : 0;
    }
}

Feedback

Please report any documentation errors, typos or suggestions to the Gendarme Google Group (http://groups.google.com/group/gendarme). Thanks!

Mono

Gendarme.Rules.Concurrency

Table of Contents

Rules

DoNotLockOnThisOrTypesRule

DoNotLockOnWeakIdentityObjectsRule

DoNotUseLockedRegionOutsideMethodRule

DoNotUseMethodImplOptionsSynchronizedRule

DoubleCheckLockingRule

NonConstantStaticFieldsShouldNotBeVisibleRule

ProtectCallToEventDelegatesRule

ReviewLockUsedOnlyForOperationsOnVariablesRule

WriteStaticFieldFromInstanceMethodRule

Feedback