0

I have a very similiar question asked already 2012.

Critical Sections and return values in C++

I'd like to access a container thread safe aswell but instead return a cached version by reference.

struct Container {
  const Data& getSomeData() const {
    EnterCriticalSection(& myCritSec);
    if (update) {
      cache.calulatefromcontainer();        
    }
    // fill retobj with data from structure
    LeaveCriticalSection(& myCritSec);
    return cache;
  }

 private:
  mutable Data cache;
};

The problem is, that "return cache" line isn't protected anymore. Is it possible to return "cache" thread safe by reference?

Community
  • 1
  • 1
  • 2
    Returning a reference to modifiable data is basicaly a no-go for thread-safe containers. Not much to depate here. – SergeyA Apr 05 '16 at 20:33

1 Answers1

0

You have to think what your critical section is actually protecting.

In your code, it looks like myCritSec is protecting the container. But notably, it is not protecting the cache member variable. That is not because the return cache; line but because you are returning a reference to it, so it could be used unrestricted by client code while other thread calls getSomeData() again and modifies it.

One solution would be to return a copy of the data.

Another solution would be that every public function usable to get information from Data will somehow use the myCritSec of the parent container. The problem with this approach is that it would be very easy to fall into races. For example:

class Data
{
public:
     int getA() const
     {
         int res;
         EnterCriticalSection(parentCS);
         res = getAunlocked();
         LeaveCriticalSection(parentCS);
         return res;
     }
     int getB() const
     {
         int res;
         EnterCriticalSection(parentCS);
         res = getBunlocked();
         LeaveCriticalSection(parentCS);
         return res;
     }
};

And then in the user code:

const Data &data = container.getSomeData();
if (data.getA() == data.getB()) // <--- RACE!!!
{
}

Since the call to getA() and getB() are each locking and unlocking the CS, another thread might modify the data just in between and create a race condition.

rodrigo
  • 94,151
  • 12
  • 143
  • 190
  • So the only real safe solution would be a copy of the data? – vcd user Apr 05 '16 at 20:43
  • @vcduser: "the only real safe solution"? There are a lot of ways of doing things right. For example, you can add the CS handling code to the user of the container, if you can trust it, for example. Or you can copy subsets of the data, or you can use copy-on-write techniques, shared pointers... – rodrigo Apr 05 '16 at 20:46