views:

257

answers:

4

Is it necessary to protect access to a single variable of a reference type in a multi-threaded application? I currently lock that variable like this:

private readonly object _lock = new object();
private MyType _value;
public MyType Value
{
  get { lock (_lock) return _value; }
  set { lock (_lock) _value = value; }
}

But I'm wondering if this is really necessary? Isn't assignment of a value to a field atomic? Can anything go wrong if I don't lock in this case?

P.S.: MyType is an immutable class: all the fields are set in the constructor and don't change. To change something, a new instance is created and assigned to the variable above.

A: 

it all depends on whether the property will be accessed by multiple threads. and some variable is said to be atomic operation, in this atomic operation case, no need to use lock. sorry for poor english.

in you case, immutable, i think lock is not necessary.

Benny
Whether the object is immutable is not relevant. The *variable* is mutable; that's why they're called "variables".
Eric Lippert
-1 because Eric is right.
tony
+6  A: 

There is the volatile keyword for this. Whether it's safe without it depends on the scenario. But the compiler can do funny stuff, such as reorganize order of operation. So even read/write to one field may be unsafe.

ewernli
hey, your link to volatile is incorrect...
despart
My mistake. Corrected the link.
ewernli
I'll add, though: `volatile` won't fix the issue in all cases.
Craig Stuntz
C# is not my specialty, but IIUC volatile _will_ work in this case. volatile (for C#) does acquire memory ordering on reads and release on writes, and that's what you need here.
tony
Tony, volatile will help with writing *the pointer itself* but not with assumptions about the version of the structure it points at. See, e.g., my reply to your other comment.
Craig Stuntz
The C# compiler never reorders anything; it's the *processor* that you've got to worry about.
Eric Lippert
The CIL may not be reordered, but can't the JIT do some reordering optimization? See this example: http://stackoverflow.com/questions/133270/how-to-illustrate-usage-of-volatile-keyword-in-c/1284007#1284007
SelflessCoder
+4  A: 

It can be an issue. It's not just the assignment itself you have to be concerned with. Due to caching, concurrent threads might see an old version of the object if you don't lock. So whether a lock is necessary will depend on precisely how you use it, and you don't show that.

Here's a free, sample chapter of "Concurrent Programming in Windows" which explains this issue in detail.

Craig Stuntz
It is not 'caching'. It is reorder of instructions in the pending read/write buffers. Similar but not the same thing. The caches in most CPUs are coherent. It is the order in which values get written to even the first cache.
tony
Tony, you're right that instruction rewriting is involved, and wrong that caching isn't. With multiple CPUs, each has their own cache, which is not guaranteed to be consistent outside of a lock. See, e.g.: http://www.bluebytesoftware.com/blog/2009/10/20/FalseSharingIsNoFun.aspx
Craig Stuntz
Sorry; wrong link. Try http://www.bluebytesoftware.com/blog/2009/05/17/LoadsCannotPassOtherLoadsRevisited.aspx
Craig Stuntz
+9  A: 

Being atomic is rarely enough.

I generally want to get the latest value for a variable, rather than potentially see a stale one - so some sort of memory barrier is required, both for reading and writing. A lock is a simple way to get this right, at the cost of potentially losing some performance due to contention.

I used to believe that making the variable volatile would be enough in this situation. I'm no longer convinced this is the case. Basically I now try to avoid writing lock-free code when shared data is involved, unless I'm able to use building blocks written by people who really understand these things (e.g. Joe Duffy).

Jon Skeet
you r right. my answer is not correct.
Benny
Also, as you yourself say in http://www.yoda.arachsys.com/csharp/threads/volatility.shtml, there's not even a guarantee about the assignment being atomic (depends on memory alignment)
Vinko Vrsalovic
@Jon, the "latest vs stale" is kinda nebulous and not definitive. I mean even if it is fully locked and protected, which value you get with a read is deoendant on the somewhat random behavior of the OS process switchng algorithm... It could run your reading thread just before the other "concurrent" write, or vice versa... This class of issue is outside the scope of what we generally consider a multi-threading issue. If it is important, it needs to be managed with some other mechanism... no?
Charles Bretana
That's why my current code is using the locks, to be sure. But what if being atomic is enough, and it doesn't matter if the data is slightly out-of-date (milliseconds?)? In my case, the variable is read a lot, but written only every half hour.
Tommy Carlier
I agree with Jon. Here's an article with more info: http://msdn.microsoft.com/en-us/magazine/cc163715.aspx Also, google for Joe Duffy's blog. He is an architect at MS responsible for the Parallel Extensions in .net 4. He writes a lot about no-lock and low-lock patterns. The theme, though, is that low-lock is a lot harder than it looks.
JMarsch
@Tommy: It's not necessarily a matter of being just slightly out of date. In some cases, without a memory barrier, the variable's "true" value will *never* be read again... you'd be *perpetually* out of date.
Jon Skeet
@Charles: No, it's not just that sort of out of date. As I wrote to Tommy, it's a matter of making sure that the latest value is read *at some point*. In some cases failing to have a memory barrier could mean you never see the new value. This is very much a multi-threading issue.
Jon Skeet
@Jon, I apologize for seeming dense, but by saying that "you would never see the new value" are you saying that future writes to that memory are inhibited or blcoked ? i.e., even if someone writes to that memory in a completely non-contentious way 10 secs later, that that write op would somehow fail? Or are you saying that future changes to that memory are somehow "hidden" from reads?
Charles Bretana
ahhhh To all, THANKS FOR ADDING comment edit capability!!!!
Charles Bretana
@Charles: I'm saying that you may not be reading from the memory you think you're reading from. You may be reading from a register whose value originally came from main memory. The rules for what you're guaranteed are easy to read but hard to really understand IMO.
Jon Skeet
@Jon, but are you not are also saying (?) that by not locking the concurrent operation, that this condition you're describing (wrong value in a register) will persist indefinitely? That is what I am questioning.. Why wouldn't a future write operation cause that register value to be overwritten with the updated value?
Charles Bretana
@Jon, (con't)... Obviously, If a future write op is not guaranteed to overwrite any register copies of the memory location the register is tied to, then that issue is a more fundamental and serious flaw than any multi-threading issue, and just locking the memory from concurrent operations will not fix it...
Charles Bretana
@Charles: I'm talking about the situation where one thread is repeatedly reading a value, whereas a different thread is writing to it. The writing thread writes to main memory, the reading thread keeps reading from the register. Locking helps because it sets up memory barriers to avoid this sort of thing. I would strongly recommend reading Joe Duffy's articles on the memory model. He knows it much better than I do :)
Jon Skeet
@Jon, I have been reading Joe Duffys book (the 1000 pager). but all is clear now. You are not saying that this condifiton will exist indefinitely, you are only saying that the value the reader gets is not guaranteeed to be the latest updated value while concurrent writes are executing. Once any concurrent write thread finishes execution, that threads write value is guaranteed to have been in all representations(memory or register) at least once prior to any future reads. (Although it may get over-written by some other subsequent concurrent write)
Charles Bretana
@Charles: No, I *am* saying the condition *may* last indefinitely. The write itself doesn't trample over any caches in registers, for example - it's up to the *reading* thread to make sure that it uses a memory barrier to avoid rereading a stale value. For example, suppose you have a Boolean flag to say whether or not a loop should terminate. If you just have a spin lock on a normal variable: `while (!stop)` then that loop could hang *forever* even if another thread sets `stop` to true. Now whether volatile fixes that or not is another matter... I used to think it did. Now I'm less sure...
Jon Skeet
@Jon, The write thread has a line of code, somewhere in it's "thread of execution", (instruction sequence), that will when executed copy the updated value to the register (or trigger whatever process is responsible for that) . So absent an indefinite blocking of the write thread, (i.e., if it is allowed to run to completion), eventually the register will get updated... So, are you saying that WHEN there is no lock to enforce serialized access, that something can potentially block that write thread from completing? if so, what, and how does having an explicit lock fix that?
Charles Bretana
@Jon, (con't) ... Or are you saying that another, separate thread is responsible for updating the cached register copies, and that THAT thread can get blocked?
Charles Bretana
@Charles: No, the writing thread *can't* write it to the register, as registers are thread local. The register is "on" the reading thread. No-one updates the cached register copy - but memory barriers effectively invalidate that cached copy, so the thread would normally (with locking or explicit barriers) end up reading from main memory again. Note that the memory model isn't *described* in those terms - it's in terms of operation reordering etc. Cached reads are just one form the logical reordering can take.
Jon Skeet
@Jon, that answers my confusion... So when a thread gets context switched back into the cpu, it is carrying it's value for the register with it from the context. If another thread was going to 'update' that register value it would have to be able to reach into the threads context on the threads kernel stack to change it... In a perfect world, the OS context switch code 'should' handle this (check the register's source memory for potential updates whilst we were sleeping) ... but I'm guessing that's the missing bit that currently causes this to not be easy...
Charles Bretana
I disagree with your idea about what would happen in a perfect world. When the CPU loads a value into a register, there's no particular indication that it should just be a cache... it may well be that you only *want* it to be the immediate value, and don't care about updates. But it sounds like you're basically along the right lines in terms of what can go wrong.
Jon Skeet
Is this because the OS has no idea whether the value being loaded onto a register is being 'cached', (to eliminate future fetches from memory), or just being loaded into a register (as all fetched values are loaded into registers) in order to execute some 'normal' CPU instruction?? As i recall, it is the Compiler, not the OS or hardware that 'decides' to do this register caching trick, right? I mean from the CPU/OS level, caching can be done using the specific CPU On-Chip L1 cache memory, which is pretty fast, and is not thread-associated (will not grt loaded/unloaded as threads switch).
Charles Bretana
Well, in this case it would be a case of the JIT compiler - but memory models can come from CPU architectures as well. Different OSes have different memory models. Ultimately, the memory model is designed to perform the balancing act of being easy to program against vs performing well. It's just up to us to understand how to do things safely - and again to decide a balance between being as fast as possible but causing brain ruptures while proving a program is correct, vs overusing locks and causing contention (and still possibly problems!)
Jon Skeet