23

I have the following Lock statement:

private readonly object ownerLock_ = new object();

lock (ownerLock_)
{
}

Should I use volatile keyword for my lock variable? 

private readonly volatile object ownerLock_ = new object();

On MSDN I saw that it usually used for a field that is accessed without locking, so if I use Lock I don't need to use volatile?

From MSDN:

The volatile modifier is usually used for a field that is accessed by multiple threads without using the lock statement to serialize access.

Dor Cohen
  • 16,769
  • 23
  • 93
  • 161

2 Answers2

22

If you're only ever accessing the data that the lock "guards" while you own the lock, then yes - making those fields volatile is superfluous. You don't need to make the ownerLock_ variable volatile either. (You haven't currently shown any actual code within the lock statement, which makes it hard to talk about in concrete terms - but I'm assuming you'd actually be reading/modifying some data within the lock statement.)

volatile should be very rarely used in application code. If you want lock-free access to a single variable, Interlocked is almost always simpler to reason about. If you want lock-free access beyond that, I would almost always start locking. (Or try to use immutable data structures to start with.)

I'd only expect to see volatile within code which is trying to build higher level abstractions for threading - so within the TPL codebase, for example. It's really a tool for experts who really understand the .NET memory model thoroughly... of whom there are very few, IMO.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • Thanks for your quick reply, Lets say I'm using only Lock, it seems rare but can 2 threads that will come to the lock statement both will check ownerLock_ state at the same time and try to lock it or each other? I'm asking this question because I had problem with the locking in my code and I think it related to this issue. – Dor Cohen Jul 17 '12 at 13:13
  • @DorCohen: Only one of them will acquire the lock at a time. It will then execute the body of the `lock` statement, and when it releases the lock, the other thread will be able to acquire the lock and execute the code. – Jon Skeet Jul 17 '12 at 13:15
  • @DorCohen, it seems that your problem is not in what you're locking, but what you're attempting to do within the lock body. – Tetsujin no Oni Jul 17 '12 at 13:16
  • @DorCohen - One thing to bear in mind with locks is that you lock on an *instance* of an object, not just a field. So you need to make sure that each thread that will attempt to acquire the lock will be given the same instance of the "guard" object. This MSDN article has more information: http://msdn.microsoft.com/en-us/library/ms173179(v=VS.80).aspx – Kristian Fenn Jul 17 '12 at 13:31
  • If there was no "readonly" (but the lock itself was never actually written) would the answer still apply? How about if the lock was writable and changing for some obscure case? – crokusek Jul 09 '13 at 18:56
  • @crokusek: If the lock is writable, that's a problem in all kinds of ways. If it's not readonly, I *think* it would still be okay. – Jon Skeet Jul 09 '13 at 19:56
3

If something is readonly it's thread-safe, period. (Well, almost. An expert might be able to figure out how to get a NullReferenceException on your lock statement, but it wouldn't be easy.) With readonly you don't need volatile, Interlocked, or locking. It's the ideal keyword for multi-threading, and you should use it where ever you can. It works great for a lock object where its big disadvantage (you can't change the value) doesn't matter.

Also, while the reference is immutable, the object referenced may not be. "new object()" is here, but if it was a List or something else mutable--and not thread-safe--you would want to lock the reference (and all other references to it, if any) to keep the object from changing in two threads at once.

RalphChapin
  • 3,108
  • 16
  • 18