Why shared_ptr object works well without lock in such case?

Question

One reader thread & multi writer threads concurrently access shared_ptr object and it works well, code as below (BUT, if i modify the write line code from "=" to "reset", it will coredump while reading) :

shared_ptr.reset means coredump and "operator =" means works well ? ( I tried more than 100 times)

bool start = false;
std::mutex mtx;
std::condition_variable cond;
std::shared_ptr<std::string> string_ptr(new std::string("hello"));

int main() {
  auto read = []() {
    {
      std::cout << "readddd" << std::endl;
      std::unique_lock<std::mutex> lck(mtx);
      while (!start) {
        cond.wait(lck);
      }
    }
    for (int i = 0; i < 100000; ++i) {
      std::cout << *string_ptr.get() << std::endl;
    }
  };

  auto write = []() {
    {
      std::unique_lock<std::mutex> lck(mtx);
      while (!start) {
        cond.wait(lck);
      }
    }
    for (int i = 0; i < 100000; ++i) {
      string_ptr = std::make_shared<std::string>(std::to_string(i));
      // string_ptr.reset(new std::string(std::to_string(i))); // will coredump
    }
  };

  std::thread w(write);
  std::thread rthreads[20];

  for (int i = 0; i < 20; ++i) {
    rthreads[i] = std::thread(read);
  }

  {
    std::unique_lock<std::mutex> lck(mtx);
    start = true;
    cond.notify_all();
  }

  w.join();
  for (auto& t : rthreads) {
    t.join();
  }

  return 0;
}

coredumpe stack will be :

#0 0x00007fee5fca3113 in std::basic_ostream<char, std::char_traits >& std::operator<< <char, std::char_traits, std::allocator >(std::basic_ostream<char, std::char_traits >&, std::basic_string<char, std::char_traits, std::allocator > const&) () from /lib64/libstdc++.so.6 #1 0x00000000004039f0 in <lambda()>::operator()(void) const (__closure=0xa54f98) at test_cast.cpp:395

line "std::cout << *string_ptr.get() << std::endl;"

Both versions suffer from a race condition. `std::shared_ptr` is not magically thread-safe. If you want to share it between threads, you have to protect it with a mutex. Just that you tried it a few times is no proof: It could be that it's just very unlikely to cause errors or even that the error is impossible on your compiler/OS/CPU combination, but that is not a guarantee. — Ulrich Eckhardt, Nov 22 '21 at 09:54
@UlrichEckhardt I tried more than 100 times and found : use operator = works well every time while use reset(new xx) will definitely result to coredump. Just wonder what's the difference between the two. — oyjh, Nov 22 '21 at 09:58
@oyjh a race condition is a type of Undefined Behaviour which includes appearing to work, right up to the point of doing an important demonstration (or handing in an assignment) — Richard Critten, Nov 22 '21 at 10:00
@oyjh One of the "joys" of undefined behaviour (at least, if you have masochistic tendencies) is that there can be no visible symptoms. The basic fact is that your code has a race condition in both cases, and therefore undefined behaviour. The fact you have tested and not seen symptoms in one case but not the other is irrelevant. Testing can only prove the *presence* of a flaw in your code (e.g. a crash). Testing cannot prove the *absence* of a flaw in your code (e.g. even if no crash occurs in a zillion tests that doesn't prove the code is free of flaws). — Peter, Nov 22 '21 at 10:03
A 100 times only? For production code I've seen race conditions that happen only 1 in a million times called and that equals a product fail. Basically you have to prove something is deadlock free or stress test for days on end. "it seems to work" is just not good enough — Pepijn Kramer, Nov 22 '21 at 10:04
Oh did I forget to mention: Always lookup documentation of what you are using, if it doesn't state it is threadsafe explicitly then it isn't threadsafe — Pepijn Kramer, Nov 22 '21 at 10:05
@PepijnKramer thanks for your comments ! What I really want is if there is any workaround can awoid lock in this situation: one writer and multi readers. If i surround the std::shared_ptr object with std::atomic, will it works well? — oyjh, Nov 22 '21 at 10:12
Two things: "What I really want is ..." in comments makes for bad questions here. All relevant info should be in the question itself. Don't change that though, but ask a different one instead. However, don't focus on `shared_ptr`. Rather, focus your research on the task: One writer, multiple readers. That should give you a few ideas already. Also, I just searched for "[c++] shared_ptr thread safe" and there are hundreds of results! I now regret not voting to close this as duplicate but answering it, even though it didn't add anything new to SO. — Ulrich Eckhardt, Nov 22 '21 at 10:16
If you need seperate reader writer access, look into reader/writer locks: https://stackoverflow.com/questions/244316/reader-writer-locks-in-c. But be careful they come with their own pitfalls. — Pepijn Kramer, Nov 22 '21 at 10:54

score 6 · Accepted Answer · answered Nov 22 '21 at 10:01

Both versions suffer from a race condition. std::shared_ptr is not magically thread-safe. If you want to share it between threads, you have to protect it with a mutex. Just that you tried it a few times is no proof: It could be that it's just very unlikely to cause errors or even that the error is impossible on your compiler/OS/CPU combination, but that is not a guarantee.

If you want to see it break more often, insert switches to the thread context into the code. The simplest way is to sleep for some time, at which point the OS schedules a different thread:

    for (int i = 0; i < 100000; ++i) {
      std::string* tmp = string_ptr.get();
      std::this_thread::yield();
      std::cout << *tmp << std::endl;
    }

Note that this code enforces the context switch, but the same switch could happen at the same point spontaneously when the thread's timeslice is exceeded.

As for the why one is more likely to fail than the other, step through the instructions executed by shared_ptr's assignment operator and reset() memberfunction. Their implementation may have larger or smaller critical section, which controls how often the error shows itself.

thanks for you answer ! What I really want is if there is any workaround can awoid lock in this situation: one writer and multi readers. If i surround the std::shared_ptr object with std::atomic, will it works well? — oyjh, Nov 22 '21 at 10:16
@oyjh Thread safety is a relational property. Code isn't thread safe; two pieces of code are relatively thread safe to each other. Various forms of thread safety are more or less composable than others. The less composible the thread safety is, the more specific the exact use case has to be in order to determine if the code is thread safe. Your description (surround something in an atomic) may or may not fix your problem, because the kind of operations you are describing are not very composible. — Yakk - Adam Nevraumont, Nov 22 '21 at 19:35

Why shared_ptr object works well without lock in such case?

1 Answers1