2

I want to make std::string append function thread safe, because I am appending a specific string from different threads.

I'm a quite beginner in c++ so I'm not sure what problems this may rise.

The solution I am thinking about is instead of using somestring.append("appendthis");

Use the following code:

bool appendtsReady = true; void appendts(std::string& originalString, char* app) { while (!appendtsReady) {} appendtsReady = false; originalString.append(app); appendtsReady = true; } appendts(somestring, "appendthis"); 

I would hope that if the string is being appended the new request appendts(somestring, "appendthis_from_different_thread"); will be caught on loop until previous append is finished.

Is this solution is too naive?

4
  • 2
    Provide a lock for the object. Eg. en.cppreference.com/w/cpp/thread/mutex Commented Sep 21, 2013 at 16:07
  • 2
    A conforming compiler's optimizer is free to conclude that the while statement's conditional will never be true and remove that code. Commented Sep 21, 2013 at 16:11
  • Yes. Adding volatile to the flag might help, but a proper lock would be vastly better. Commented Sep 21, 2013 at 16:16
  • 1
    If you are "quite a beginner" you should not be writing multi-threaded code. Commented Sep 21, 2013 at 16:35

1 Answer 1

8

This is not a thread safe way. Even if appendtsReady were an atomic bool (if you don't change the logic)! Here's why:

Consider noone has ever written to the bool and two threads execute appendts. Both read the bool. Now can both read true? Yes! Because between the read of appendtsReady in the while loop and the write in the line below there is a tiny delay! So tiny it will work almost always, but the read of the second thread may come exactly during that delay so both read true.

The solution: Let them share a std::mutex.

std::mutex mutex; void appendts(std::string& originalString, char* app) { std::lock_guard<std::mutex> lock(mutex); originalString.append(app); } 

Now you can call this method from two threads, provided that both threads know about the same mutex. Either declare it globally (not so nice) or pass references of the mutex to both threads.

std::mutex works by locking and unlocking, just as you wanted with your boolean variable. However, std::mutex has thread-safety.

I recommend using a std::lock_guard instead of mutex.lock(); work(); mutex.unlock(); as it provides the goodie of RAII, that is if work() returns, throws or breaks or whatever, the mutex get's unlocked automatically.

Sign up to request clarification or add additional context in comments.

2 Comments

You could make the atomic flag version work via test-and-set. But the mutex is better.
@AlanStokes Right, what I was trying to say, that only saying "it's atomic, it's thread safe" isn't enough, because the logic would still need to be different. I've added a bit to be clearer on that, thanks!

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.