14

I've got some multi threaded code I'd like to increase the performace of a bit, so I'm wondering if I can get rid of a lock.

I've got a field member:

private IList<ServerStatus> status;

It's updated in a thread like so:

status = GetUpdatedStatus();

And it's used in another thread like this:

var currentStatus = status;

So the question is, can the above yield any problems without locks around the two assignment statements ?

I guess the only scenario I can see is currentStatus being null, but then again I'd expect an assignment to be somewhat thread-safe (either it has changed the reference or not)

xanatos
  • 109,618
  • 12
  • 197
  • 280
Steffen
  • 13,648
  • 7
  • 57
  • 67

2 Answers2

22

You are right. You will see the assignment or you won't see it. Assignments (and reads) of references are always "atomic" (in the end it's because on 32 bits machines references are 32 bits, so can be done atomically, and on 64 bits machines (running a 64 bits app) references are 64 bits, so can be done atomically. The only exception is trying to write/read a long (64 bits) on a 32 bits machine. There you would have to use Interlocked.Read / Interlocked.Exchange)

Normally should declare status as volatile, so that each thread sees only the latest version. You should read this: http://www.albahari.com/threading/ it's very very good!

If you don't trust me, read the section Do We Really Need Locks and Barriers? here http://www.albahari.com/threading/part4.aspx

Ah... I was forgetting... The world HATES you, so there is a little thing to know of volatile: sometimes it doesn't work :-) :-) Read, in the same page of the other example, the section The volatile keyword, the part UNDER the red box. Notice that applying volatile doesn’t prevent a write followed by a read from being swapped, and this can create brainteasers. In the end, the only way to be sure is to use Interlocked.Exchange to write and Interlocked.CompareExchange to read something OR protect the read and the write sections with synchronization (like lock) OR fill your program with Thread.MemoryBarrier (but don't try it, you'll fail, and you won't even know why). You are guaranteed that all the reads and the writes done in the lock will be done IN the lock, not before or after.

xanatos
  • 109,618
  • 12
  • 197
  • 280
  • Excellent answer, and thanks for the links - I've read a bit about volatile not working 100% before, so I tend to stay away from it and use memory barriers or lock instead. – Steffen Feb 23 '11 at 06:50
  • 1
    I had to work with threads, synchronizations etc the last month, so I read nearly everything there was on Internet about the argument. Threading can give nightmares if you want to tightly synchronize threads. – xanatos Feb 23 '11 at 06:54
6

Reference writes are guaranteed atomic, so there are only really two things to check:

  • usage in a tight loop - might need to add volatile if you need to notice the change
  • double updates; if you are (for example) doing add/remove via reference swap, you should use Interlocked.CompareExchange to make sure you don't lose data; keep reapplying your change until you win the swap

i.e.

object snapshot, newValue;
do
{
    snapshot = field;

    // do something based on that; create a clone
    // with more/less data for example
    newValue = ...;
} while (!ReferenceEquals(
    Interlocked.CompareExchange(ref field, newValue, snapshot), snapshot));
Marc Gravell
  • 1,026,079
  • 266
  • 2,566
  • 2,900
  • Marc I already read you saying that on SO and I'm really wondering the purpose of this technique. You say "until you win the swap", but if two threads are running your loop, how does it end? How can one win it? I don't get it. If another thread fetch the data in the meantime why not noticing it with the CompareExchange and use this data instead of the one from your thread. I know it's the opposite way of seeing things, but I don't understand how this code can work on multiple threads running it. Thanks. – Nock Aug 26 '14 at 21:39
  • @Nock compared exchange does an atomic swap if the value still matches; in the competing case at least one thread is guaranteed to be successful per iteration - if all are unsuccessful then no change is ever made, in which case they can't all be unsuccessful (contradiction) – Marc Gravell Aug 26 '14 at 22:31
  • Alright I understand now, if there're 10 threads competing, then one thread "is right" per iteration (then exit) and it'd take 10 iterations for the last thread to be successful. One last question: what is the point of this? What benefit it has compared to a raw implem that would not bother to know if the write was committed or not? I mean if your loop has no additional logic for each iteration you detect that you failed to commit and adapt to this failure then retry. To be clearer: the point is to know that you failed (and react accordingly) or to be sure you commit? (if so why?) – Nock Aug 26 '14 at 23:35
  • @Nock it doesn't *necessarily* take 10 iterations for the last thread to be successful; it depends on how they are competing, in terms of timing; they could **all** succeed first time, if the stars align. The advantage is that we haven't needed a lock object, and haven't needed to block; in most real-world scenarios, a conflict is rare, so retry would be uncommon. It is basically optimistic vs pessimistic concurrency. – Marc Gravell Aug 27 '14 at 07:42
  • @Marc, can I have your thoughts on http://stackoverflow.com/questions/31111056/why-should-i-use-spinwait ? – MaYaN Jun 29 '15 at 08:39