20

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)

Chris Barlow
  • 3,274
  • 4
  • 31
  • 52
  • 1
    I'm guessing db.TableAwesom is a public table. Could you create a public method that accesses this table instead and use your private lock inside that? –  May 24 '11 at 14:55

5 Answers5

21

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.

configurator
  • 40,828
  • 14
  • 81
  • 115
Eric Lippert
  • 647,829
  • 179
  • 1,238
  • 2,067
  • I was just suggesting the third class approach, but my typing is not as fast. :P – Calvin Fisher May 24 '11 at 14:58
  • Thanks, Eric, those are good thoughts. Let us not follow rules for the sake of following rules. =) – Chris Barlow May 24 '11 at 14:58
  • I think that if the idea is to simply make debugging the deadlock problem easier then a public mutex might help, but it's no less likely to *cause* the problem in the first place than a public lock object - you can still have other code locking on it, causing the problem. The only solution I see is to reduce the instances of accessing the resource down to a method (or at the very least a class) and lock on it there; whether using a mutex there or an object seems irrelevant at that point. Thanks everyone!!! – Chris Barlow May 24 '11 at 15:47
9

The reason it's bad practice to lock on a public object is that you can never be sure who ELSE is locking on that object. Although unlikely, someone else someday can decide that they want to grab your lock object, and do some process that ends up calling your code, where you lock onto that same lock object, and now you have an impossible deadlock to figure out. (It's the same issue for using 'this').

A better way to do this would be to use a public Mutex object. These are much more heavyweight, but it's much easier to debug the issue.

DanTheMan
  • 3,277
  • 2
  • 21
  • 40
  • Thank you for the explanation on why it's a bad idea -- I was a little fuzzy on the reasoning. =) – Chris Barlow May 24 '11 at 14:54
  • 2
    This answer is also a good explanation of that point: http://stackoverflow.com/questions/1873201/why-are-locks-performed-on-separate-objects – Calvin Fisher May 24 '11 at 14:54
2

Use a Mutex.
You can create mutex in main class and call Wait method at the beginning of each class (method); then set mutex so when the other method is called it gonna wait for first class to finish.
Ah, remember to release mutex exiting from those methods...

Marco
  • 56,740
  • 14
  • 129
  • 152
  • 4
    How is locking with a public mutex any better (or worse) than locking on a public object? – Eric Lippert May 24 '11 at 14:57
  • @Eric: I suggested `Mutex` too, I considered that because we can 'wait' it gives us control of flow rather than potentially breaking outright, or we can use `ReaderWriterLockSlim` with `TryEnter` and such-and-such - ultimately giving access to a 'state', I guessed would help - I take it you consider otherwise? Only asking because I'd remove my bad answer if so. – Grant Thomas May 24 '11 at 15:01
  • @Mr. Disappointment: Let me clarify; I was not intending to criticize Marco's answer; rather it was an honest question. Someone reading this answer might well be confused: if you can use a lock or use a mutex then what are the pros and cons of each? – Eric Lippert May 24 '11 at 15:04
  • @Eric,@Mr. Disappointment: Mutex is used to synchronize concurrent threads, so this seems to me the "natural environment" to use a mutex in. But this is my opinions and I could be wrong, so if you teach me a better way I say thank you!! :) – Marco May 24 '11 at 15:07
  • @Eric: I suppose the deciding factor would be down to which aids best in problem prevention (or mitigation when coming to human error.) – Grant Thomas May 24 '11 at 15:07
1

I see two differing questions here:

Why is it a bad idea to lock on a public object?

The idea is that locking on an object restricts access while the lock is maintained - this means none of its members can be accessed, and other sources may not be aware of the lock and attempt to utilise the instance, even trying to acquire a lock themselves, hence causing problems.

For this reason, use a dedicated object instance to lock onto.

How do I lock these methods so that they don't run in tandem?

You could consider the Mutex class; creating a 'global' mutex will allow your classes to operate on the basis of knowing the state of the lock throughout the application. Or, you could use a shared ReaderWriterLockSlim instance, but I wouldn't really recommend the cross-class sharing of it.

Grant Thomas
  • 44,454
  • 10
  • 85
  • 129
0

You can use a public LOCK object as a lock object. You'll just have to specify that the object you're creating is a Lock object solely used for locking the Ninja and Pirate class.

CodingBarfield
  • 3,392
  • 2
  • 27
  • 54