Gendarme.Rules.Concurrency.DoNotUseLockedRegionOutsideMethodRule(git)

DoNotUseLockedRegionOutsideMethodRule

Assembly: Gendarme.Rules.Concurrency
Version: git

Description

This rule will fire if a method calls System.Threading.Monitor.Enter, but not System.Threading.Monitor.Exit. This is a bad idea for public methods because the callers must (indirectly) manage a lock which they do not own. This increases the potential for problems such as dead locks because locking/unlocking may not be done together, the callers must do the unlocking even in the presence of exceptions, and it may not be completely clear that the public method is acquiring a lock without releasing it. This is less of a problem for private methods because the lock is managed by code that owns the lock. So, it's relatively easy to analyze the class to ensure that the lock is locked and unlocked correctly and that any invariants are preserved when the lock is acquired and after it is released. However it is usually simpler and more maintainable if methods unlock whatever they lock.

Examples

Bad example:

class BadExample {
    int producer = 0;
    object lock = new object();
    // This class is meant to be thread safe, but in the interests of
    // performance it requires clients to manage its lock. This allows
    // clients to grab the lock, batch up edits, and release the lock
    // when they are done. But this means that the clients must
    // now (implicitly) manage the lock which is problematic, especially
    // if this object is shared across threads.
    public void BeginEdits ()
    {
        Monitor.Enter (lock);
    }
    public void AddProducer ()
    {
        // Real code would either assert or throw if the lock is not held.
        producer++;
    }
    public void EndEdits ()
    {
        Monitor.Exit (lock);
    }
}

Good example:

class GoodExample {
    int producer = 0;
    object mutex = new object();
    public void AddProducer ()
    {
        // We need a try block in case the assembly is compiled with
        // checked arithmetic.
        Monitor.Enter (mutex);
        try {
            producer++;
        }
        finally {
            Monitor.Exit (mutex);
        }
    }
    public void AddProducer2 ()
    {
        // Same as the above, but with C# sugar.
        lock (mutex) {
            producer++;
        }
    }
}

Source code

You can browse the latest source code of this rule on github.com