C++/MFC/ATL Thread-Safe String read/write

2.1k views Asked by At

I have a MFC class with threads launched and the threads need to modify CString members of the main class.

I hate mutex locks, so there must be a an easier way to do this.

I am thinking to use the boost.org library or atl::atomic or shared_ptr variables.

What is the best method of reading and writting the string and be thread safe?

class MyClass
{
   public:
      void        MyClass();
      static UINT MyThread(LPVOID pArg);
      CString     m_strInfo;
};

void MyClass::MyClass()
{
    AfxBeginThread(MyThread, this);
    CString strTmp=m_strInfo; // this may cause crash
}

UINT MyClass::MyThread(LPVOID pArg)
{
     MyClass pClass=(MyClass*)pArd;
     pClass->m_strInfo=_T("New Value"); // non thread-safe change
}

According to MSDN shared_ptr works automatically https://msdn.microsoft.com/en-us/library/bb982026.aspx

So is this a better method?

#include <memory>
class MyClass
{
   public:
      void        MyClass();
      static UINT MyThread(LPVOID pArg);
      std::shared_ptr<CString>    m_strInfo;  // ********
};

void MyClass::MyClass()
{
    AfxBeginThread(MyThread, this);
    CString strTmp=m_strInfo; // this may cause crash
}

UINT MyClass::MyThread(LPVOID pArg)
{
     MyClass pClass=(MyClass*)pArd;
     shared_ptr<CString> newValue(new CString());

     newValue->SetString(_T("New Value"));
     pClass->m_strInfo=newValue; // thread-safe change?
}
2

There are 2 answers

6
marcinj On BEST ANSWER

You could implement some kind of lockless way to achieve that, but it depends on how you use MyClass and your thread. If your thread is processing some data and after processing it, it need to update MyClass, then consider putting your string data in some other class ex.:

struct StringData {
    CString     m_strInfo;
};

then inside your MyClass:

class MyClass
{
   public:
      void        MyClass();
      static UINT MyThread(LPVOID pArg);
      StringData*     m_pstrData;
      StringData*     m_pstrDataForThreads;
};

now, the idea is that in your ie. main thread code you use m_pstrData, but you need to use atomics to store local pointer to it ie.:

void MyClass::MyClass()
{
    AfxBeginThread(MyThread, this);

    StringData*  m_pstrDataTemp = ATOMIC_READ(m_pstrData);
    if ( m_pstrDataTemp )
        CString strTmp=m_pstrDataTemp->m_strInfo; // this may NOT cause crash
}

once your thread finished processing data, and wants to update string, you will atomically assign m_pstrDataForThreads to m_pstrData, and allocate new m_pstrDataForThreads,

The problem is with how to safely delete m_pstrData, I suppose you could use here std::shared_ptr.

In the end it looks kind of complicated and IMO not really worth the effort, at least it is hard to tell if this is really thread safe, and when code will get more complicated - it will still be thread safe. Also this is for single worker thread case, and You say you have multiple threads. Thats why critical section is a starting point, and if it is too slow then think of using lockless approach.

btw. depending on how often you string data is updated you could also think about using PostMessage to safely pass a pointer to new string, to your main thread.

[edit]

ATOMIC_MACRO does not exists, its just a place holder to make it compile use ie. c++11 atomics, example below:

#include <atomic>
...
std::atomic<uint64_t> sharedValue(0);
sharedValue.store(123, std::memory_order_relaxed);          // atomically store
uint64_t ret = sharedValue.load(std::memory_order_relaxed); // atomically read
std::cout << ret;
0
Ajay On

I would have used simpler approach by protecting the variable with a SetStrInfo:

void SetStrInfo(const CString& str)
{
    [Lock-here]
    m_strInfo = str;
    [Unlock-here]
}

For locking and unlocking we may use CCriticalSection (member of class), or wrap it around CSingleLock RAII. We may also use slim-reader writer locks for performance reasons (wrap with RAII - write a simple class). We may also use newer C++ techniques for RAII locking/unlocking.

Call me old-school, but for me std namespace has complicated set of options - doesn't suit everything, and everyone.