3

EDIT: As it turns out when I was browsing I found a question the appears to be the same as mine which I didn't find earlier: Difference between lock(locker) and lock(variable_which_I_am_using)

I am looking at some code and trying to get my head around the locking thing and I am getting there I think.

Now I noticed in some code I am reviewing that an object is created like so:

private HashSet<Graphic> clustersInUse = new HashSet<Graphic>();

Then further in the code is used like so:

lock (clustersInUse)
{
   // Do something with the Hashset
}

Now, is there a problem doing this rather than creating a specific object for the lock. Like this:

private object clusterLocker = new object();

What happens if the clustersInUse above somehow gets put into a public property, what happens then?

Also, if something tries to access the clustersInUse without locking it whilst it is locked in another thread what would happen then?

Community
  • 1
  • 1
Firedragon
  • 3,685
  • 3
  • 35
  • 75
  • The simplest reason you should never lock on the object you intend to use is that you cannot guarantee that the object you are locking on does not use `lock(this)` internally. It _shouldn't_ do that since it's bad, but there is nothing preventing it so it's not safe. – AnorZaken Mar 03 '19 at 16:02

2 Answers2

3

The general rule is that you want to control the scope of the object your locking on to prevent some unknown code from causing unexpected behavior. In this case you are using a private instance variable so you are probably OK as long as you are not handing out references to it.

If you are handing out references and locking on it and other code is locking those references (e.g. when modifying the collection) changing the behavior could easily introduce threading bugs.

If someone puts it into a public property that counts as "handing out references" if they lock on it your call to lock will block until they unlock it. Whether this is desirable or not depends on what they are doing with the collection.

Having the object locked will have have no effect on using the object for any purpose other than synchronization.

Yaur
  • 7,333
  • 1
  • 25
  • 36
2

You've pretty much answered your own question. For locking, it's generally better to create an object specifically for the purpose, and usually held privately for use by accessor methods that express synchronisation logic at a high level.

Marcelo Cantos
  • 181,030
  • 38
  • 327
  • 365
  • So basically all the issues or potential issues is why on the whole it is always better to just create a specific locking object? Are there any cases where it is okay to use the object itself? Maybe for performance or something? – Firedragon Nov 21 '11 at 09:08
  • 2
    @Firedragon: It's always OK *not* to use the object itself. Don't complicate life. – Marcelo Cantos Nov 21 '11 at 09:23