Is this code thread-safe?

Question

This is a simplified version of some code I'm currently maintaining:

int SomeFunc() 
{
  const long lIndex = m_lCurrentIndex;
  int nSum = 0;
  nSum += m_someArray[lIndex];
  nSum += m_someArray[lIndex];
  return nSum;
}

lCurrentIndex is updated periodically by another thread. The question is; will making a local copy of m_CurrentIndex make sure both accesses to m_someArray uses the same index?

Please note that this is a simplified example; I'm thinking about the concept of making a local copy, not the exact piece of code shown here. I know the compiler will put the value in a register, but that is still a local copy as opposed to reading from lCurrentIndex twice.

Thanks!

Edit: The initial assignment is safe, both are guaranteed to be 32 bit in our setup. Edit2: And they are correctly aligned on a 32bit boundary (forgot about that one)

Answer 1

will making a local copy of m_CurrentIndex make sure both accesses to m_someArray uses the same index?

In the same execution of SomeFunc, yes, of course. A local integer variable (lIndex) will not magically change its value in the middle of the function.

Of course, the following are also true: the actual value of m_someArray[lIndex] (as opposed to that of lIndex) might change; m_someArray in itself might change; and what Neil said about the validity of lIndex's initial value.

Answer 2

Yes the copy of the current index will make sure both array accesses use the same index. That is not really what I would have thought of as "thread safe" though. You need to concern yourself with concurrent access to shared variables. To me that looks like the access to the array could be an area of potential concern as well.

Answer 3

No, the initialisation of the local, which reads a shared variable, is not necessarily atomic. (consider what code would be needed on an 8-bit platform, for example) In general, the only way to write thread safe code is to use atomic operations specified by your compiler and/or OS, or to use OS locking features.

Answer 4

Yes, it will access the same element of the array. It is like you are taking a snapshot of the value of m_lCurrentIndex into the local variable. Since the local variable has its own memory whatever you do to m_lCurrentIndex will have no effect on the local variable. However, note that since the assignment operation is not guaranteed to be atomic you may very well end up with an invalid value in lIndex. This happens if you update m_lCurrentIndex from one thread and at the same time try the assignement into lIndex from the other thread.

Answer 5

Another question to ask here is: is the copy operation from m_lCurrentIndex to lIndex an atomic operation? If it isn’t you might end up using very weird values which will probably do nothing good. :)

Point is: when you are using multiple threads there won’t be a way around some kind of locking or synchronization.

Answer 6

Concept of local copy

I'm thinking about the concept of making a local copy, not the exact piece of code shown here.

This question cannot be answered without knowing more details. It boils down into the questions if this "making a local copy" of m_lCurrentIndex into lIndex is atomic.

Assuming x86 and assuming m_lCurrentIndex is DWORD aligned and assuming long represents DWORD (which is true on most x86 compilers), then yes, this is atomic. Assuming x64 and assuming long represents DWORD and m_lCurrentIndex is DWORD aligned or long represents 64b word and m_lCurrentIndex is 64b aligned again yes, this is atomic. On other platforms or without the alignment guarantee two or more physical reads may be required for the copy.

Even without local copy being atomic you still may be able to make it lock-less and thread safe using CAS style loop (be optimistic and assume you can do without locking, after doing the operation check if everything went OK, if not, rollback and try again), but it may be a lot more work and the result will be lock-less, but not wait-less.

Memory barries

A word of caution: once you will move one step forward, you will most likely be handling multiple variables simultaneously, or handling shared variables which act as pointers or indices to access other shared variables. At that point things will start more and more complicated, as from that point you need to consider things like read / write reordering and memory barriers. See also How can I write a lock free structure

Answer 7

This should be thread safe (at least it will on all the compilers/OSes I've worked with). However, to be extra doubly sure you could declare m_lCurrentIndex as volatile. Then the compiler will know that it might change at any time.