C++でstd::stringをスレッドセーフにする

c++を使っている時マルチスレッドで一方は文字列の更新、もう一方は更新した文字列のコピー取得をしていたのですがおかしな動きをすることがありまして、例えば、以下の動きを確認すると

#include <iostream>
#include <mutex>
#include <string>
#include <thread>

int main(int argc, char *argv[]) {
  std::string update;
  std::string copy;

  auto updateFunc = [&update]() {
    bool a = true;
    while (true) {
      if (a) {
        update = std::move("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");
      } else {
        update = std::move("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb");
      }
      a = !a;
    }
  };

  auto copyFunc = [&update, &copy]() {
    while (true) {
      copy = update;
      if (copy != "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" &&
          copy != "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb") {
        std::cout << copy << std::endl;
    }
  };

  std::thread t1(updateFunc);
  std::thread t2(copyFunc);
  t1.join();
  t2.join();
}

CentOS Linux release 8.4.2105でlibstdc++.so.6を使っている場合、以下のように中途半端な状態でのコピーが確認できました。これを実行すると以下のように中途半端な状態でのコピーが確認できます。

aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbb
bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbaaaaaaaaaa
bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbb
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbb
bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbaaaaaaaaaa
bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbb
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbb

つまり、ヒープ上にある文字列の実体更新中にコピーを取るので中途半になるのかと思われます。

こちらの対応として、更新とコピーを同時に行わないようロックを取ることが考えられるので以下のように修正してみます。

#include <iostream>
#include <mutex>
#include <string>
#include <thread>

class spinlock {
 private:
  std::atomic_flag state_;

 public:
  spinlock() : state_(ATOMIC_FLAG_INIT) {}

  void lock() {
    // 現在の状態をロック状態にする
    while (state_.test_and_set(std::memory_order_acquire)) {
      // busy-wait...アンロックされるまで待機
    }
  }

  void unlock() {
    // 値をアンロック状態にする
    state_.clear(std::memory_order_release);
  }
};

class lockedstr {
 public:
  lockedstr() : val_("") {}
  lockedstr(std::string val) : val_(val) {}

  std::string val_;
  spinlock lock_;
  void lock() { lock_.lock(); }
  void unlock() { lock_.unlock(); }

  lockedstr &operator=(const std::string val) {
    std::lock_guard<lockedstr> lk(*this);
    val_ = std::move(val);
    return *this;
  }

  lockedstr &operator=(lockedstr &val) {
    std::lock_guard<lockedstr> lk(val);
    val_ = val.val_;
    return *this;
  }
};

int main(int argc, char *argv[]) {
  lockedstr update;
  lockedstr copy;

  auto updateFunc = [&update]() {
    bool a = true;
    while (true) {
      if (a) {
        update = std::move("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");
      } else {
        update = std::move("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb");
      }
      a = !a;
    }
  };

  auto copyFunc = [&update, &copy]() {
    while (true) {
      copy = update;
      if (copy.val_ != "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" &&
          copy.val_ != "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb") {
        std::cout << copy.val_ << std::endl;
      }
    }
  };

  std::thread t1(updateFunc);
  std::thread t2(copyFunc);
  t1.join();
  t2.join();
}

この修正で中途半端な状態でのコピーは防げることが確認できました。具体的にロックを取るところですが、以下のようになっていまして

  lockedstr &operator=(const std::string val) {
    std::lock_guard<lockedstr> lk(*this);
    val_ = std::move(val);
    return *this;
  }

  lockedstr &operator=(lockedstr &val) {
    std::lock_guard<lockedstr> lk(val);
    val_ = val.val_;
    return *this;
  }

それぞれ更新時に更新対象でロックを取るようにし、コピー作成時はコピー元でロックを取るようにしています。今回の使い方では更新用のスレッドが一つで、コピー作成用のスレッドが一つなので、それであればこのロックの取り方で大丈夫かと思われます。ただコピー作成用のスレッドが2つ以上になってくるとコピー先でもロックを取るなど適切にロックをとれるよう修正が必要そうです。