c# - Why Locking On a Public Object is a Bad Idea

Question

Ok, I've used locks quite a bit, but I've never had this scenario before. I have two different classes that contain code used to modify the same MSAccess database:

public class DatabaseNinja
{
    public void UseSQLKatana
    {
        //Code to execute queries against db.TableAwesome
    }
}

public class DatabasePirate
{
    public void UseSQLCutlass
    {
        //Code to execute queries against db.TableAwesome
    }
}

This is a problem, because transactions to the database cannot be executed in parallel, and these methods (UseSQLKatana and UseSQLCutlass) are called by different threads.

In my research, I see that it is bad practice to use a public object as a lock object so how do I lock these methods so that they don't run in tandem? Is the answer simply to have these methods in the same class? (That is actually not so simple in my real code)

Answer 1

Well, first off, you could create a third class:

internal class ImplementationDetail
{
    private static readonly object lockme = new object();
    public static void DoDatabaseQuery(whatever)
    {
        lock(lockme)
             ReallyDoQuery(whatever);
    }
}

and now UseSQLKatana and UseSQLCutlass call ImplementationDetail.DoDatabaseQuery.

Second, you could decide to not worry about it, and lock an object that is visible to both types. The primary reason to avoid that is because it becomes difficult to reason about who is locking the object, and difficult to protect against hostile partially trusted code locking the object maliciously. If you don't care about either downside then you don't have to blindly follow the guideline.