Is it Thread Safe to read from lower index elements of an struct array while it is being filled with data in main thread

Viewed 177

Original Question:

I got an array of structs and filling it in a separate thread while reading it in main thread:

struct DataModel MyData[1024];

struct DataModel
{
    bool IsFilled;
    float a;
    float b;
}
  • I have a Thread which is filling the Mydata array from 0 index to the last index(in above it is 1024).

  • Then i get the last filled struct index out from the filling thread.

  • And then i try to read the the values of the element with one index lower than the filled one.

  • Let 's assume that when the 500th element is filled i read the value from 499 element of MyData array, so i assure that i am not reading the array element which is being written.

Q1: Is this thread safe?

Q2: is there chance of occurring undefined behavior or misreading the vales?


Further Edits:

The question was improperly edited to add more details and that's why it introduced inconsistency over answers so i separated the previous edits to improve consistency over answers and accepted answer.

Edit 1: this is a suggestion of the possible implementation. though it may show false results, but just i wanted to ask about thread-safety and and undefined behavior, the following solution may show various result but i was trying to ask about thread safety first.

std::atomic<int> FilledIndex;
    void FillingMyData(struct DataModel myData[])
    {
      for(size_t i = 0; i < 1024; i++)
      {
        myData[i].a = rand();
        myData[i].b = rand();
        myData[i].IsFilled = true;

    FilledIndex = i;
  }
}

int main()
{
     std::thread ReadThread(FillingMyData, MyData);
     while(FilledIndex < 1024)
     {
          std::cout << MyData[FilledIndex].a;
     }
     ReadThread.join();
     return 0;
}
4 Answers

Yes, it is safe to work on separate objects in the same array. Even though an array is an object, it is the elements of the array that we are working on and those are separate objects. As long as you don't read an element the writer is writing to, then there is no data race and the code has defined behavior. You do have synchronization issues with your posted code but the other answers here cover hat those are.

What can happen here though is what is called false sharing. What happens in these cases is the separate objects are in the same cache line in memory. When on core/thread changes one object, that cache line gets marked as changed. This means the other core/thread has to re-sync that line to pull in any changes which means both cores/threads can't run at the same time. This is a performance penalty only though, and program will still give the correct results, it will just be slower.

The code has a data race and will continue to loop forever.

The data race happens because the initial value of FilledIndex is 0 so, the first time through the loop, you are reading from the same index to which you are writing (because i == 0).

The loop will never end because i will never reach the terminating value - the loop will exit before setting FilledIndex to 1024.

The code as written is not necessarily safe, and may not do anything useful.

  • the initial value of FilledIndex is zero, so it can read data from index zero before writing, including possibly partially written values. You probably want to set it to -1 and wait for it to be >= 0 before output.
  • there is nothing to stop the main thread executing indefinitely, outputting the same value at zero forever - it's up to the scheduler; there also is nothing to stop the main thread outputting the value at a given index more than once. You probably want to count from zero to filled index rather than outputting the value at the filled index.

The above concerns may mean that you choose a different means to communicate the value of filled index back to the main thread.

I think that this code is definitely not thread-safe.

First of all the variable FilledIndex is not initialized: as stated by cplusplus.com if you don't submit any value to the constructor, the atomic variable is left in an uninitialized state. This can lead to unexpected behaviors.

Another problem is with the exit condition in the main thread, because the for statement in ReadThread loops until 1023, so FilledIndex will never assume the value 1024 and the main thread never exits.

But the main problem is the unpredictability of the scheduling of your threads: what assures that the ReadThread is executed after the main one? Nothing!

So you cannot be sure that you're looping on all values of the array. In fact, if you try to execute many times your program, you'll see that the output every time is different and different values of the array are printed out.

For example, if we name ReadThread as T, the main thread as M and the array as A, these are possible schedulings (assuming A of size 5 for semplicity):

  • T T T M T the output will be A[2]
  • M M T M T the output will be A[0] A[0] A[1]

In fact, you're printing A[FilledIndex] and you cannot predict how FilledIndex will be updated, because it depends from thread scheduling.

I hope that you'll understand what I'm trying to say. For any questions or clarifications, obviously I'm here! I'll reply as soon as possible!

Related