Is this C++ pointer usage thread safe?

Question

Do I need insert fence before "p = tmp" to avoid memory reordering? Is it possible "p=tmp" executed before "(*tmp)[1]=2" due to memory reordering from the view of thread 2 without using fence/atomic/mutex?

thread 1

extern const std::map<int, int>* p;
auto tmp = new std::map<int, int>;
(*tmp)[1] = 2;
...
(*tmp)[2] = 3;
// do I need insert fence here to make sure above operation on tmp ready before tmp assigned to p?
p = tmp;

thread 2

extern const std::map<int, int>* p; // suppose p initalized with {{1:2}}
assert(p->find(1)->second == 2);

You should always guard your shared resource if one thread writes to it while another might want to access it. Critical section is one instruction here, so no much overhead. — Yksisarvinen, Jun 29 '18 at 08:30
Possible duplicate of [Is pointer assignment atomic in C++?](https://stackoverflow.com/questions/8919818/is-pointer-assignment-atomic-in-c) — VLL, Jun 29 '18 at 08:31
this isn't safe, you can make your pointer an atomic or use locks. — Tyker, Jun 29 '18 at 08:33
This is undefined behaviour in c++, no amount of fences will save you. — Mike Vine, Jun 29 '18 at 08:34
@FantasticMrFox fences are completely architecture dependent, without knowing the architecture we can't know if he needs one or two fences. and they are most of the time overkill — Tyker, Jun 29 '18 at 09:42

Tyker · Answer 1 · 2018-06-29T09:16:36.147

Is it possible "p=tmp" executed before "(*tmp)[1]=2" due to memory reordering from the view of thread 2 without using fence/atomic/mutex?

can it happen yes

volatile should be added to the list of things preventing the reordering but volatile would still not protect from the data race

Do I need insert fence before "p = tmp" to avoid memory reordering?

you need to add a synchronization but fences are often suboptimal and were they are needed is architecture specific. atomic would be better suited in that case

with #include <atomic>

thread 1

extern std::atomic<const std::map<int, int>*> p;
auto tmp = new std::map<int, int>;
(*tmp)[1] = 2;
...
(*tmp)[2] = 3;
// do I need insert fence here to make sure above operation on tmp ready before tmp assigned to p?
p = tmp;

thread 2

extern std::atomic<const std::map<int, int>*> p; // suppose p initalized with {{1:2}}
assert(p->find(1)->second == 2);

I prefer to use std::atomic_thread_fence(std::memory_order_release) before p = tmp for best performance. — poordeveloper, Jun 29 '18 at 18:15

wtom · Answer 2 · 2018-06-29T08:54:09.367

You have thread 2 to wait for thread 1 to complete its chunk of work

thread 1

extern const std::map<int, int>* p;
auto tmp = new std::map<int, int>;
(*tmp)[1] = 2;
...
(*tmp)[2] = 3;
p = tmp;
// set event here

thread 2

extern const std::map<int, int>* p; // suppose p initalized with {{1:2}}
// wait for event here
assert(p->find(1)->second == 2);

or you can guard p with CS or mutex in both threads but then p should be checked for validity before usage in thread 2

thread 1

extern const std::map<int, int>* p;
auto tmp = new std::map<int, int>;
// lock mutex here
(*tmp)[1] = 2;
...
(*tmp)[2] = 3;
p = tmp;
// unlock mutex here

thread 2

extern const std::map<int, int>* p; // suppose p initalized with {{1:2}}
// lock mutex here
// check if p is initialized:
// if (p is ok){
    assert(p->find(1)->second == 2);
// }
// unlock mutex here

Is this C++ pointer usage thread safe?

2 Answers2