New locker mechanism and small data race fixes#1166
New locker mechanism and small data race fixes#1166mkornaukhov wants to merge 8 commits intomasterfrom
Conversation
mkornaukhov
force-pushed
the
mkornaukhov03/tsan-compiler
branch
from
4d08728 to
7478704
Compare
mkornaukhov
force-pushed
the
mkornaukhov03/tsan-compiler
branch
from
cd7d4ed to
725f1a0
Compare
mkornaukhov
force-pushed
the
mkornaukhov03/tsan-compiler
branch
from
1dfc11e to
650367c
Compare
mkornaukhov
changed the title
| } | ||
|
|
||
| copyable_atomic_integral& operator=(copyable_atomic_integral other) { | ||
| *this = other.load(); |
There was a problem hiding this comment.
It looks like risky. Does it potentially lead to recursive invocation of assign operator?
I suppose, will be better to rewrite
if (this != &other) {
this->store(other.load());
}
return *this;
Correct me, if I'm wrong.
| std::atomic<T>(other.load()) { | ||
| } | ||
|
|
||
| copyable_atomic_integral& operator=(copyable_atomic_integral other) { |
There was a problem hiding this comment.
Maybe, will be better to pass by reference?
There was a problem hiding this comment.
No, it is intentional. It handles self assignment problem properly: currently in 56 line will be used constructor number 2. In case of your suggestions there will be problem with self assignment because of another order of function (constructor in this case) overloading.
|
|
||
| void set_functions_txt_parsed() { | ||
| is_functions_txt_parsed = true; | ||
| is_functions_txt_parsed.store(true, std::memory_order_seq_cst); |
There was a problem hiding this comment.
Why do we use certain memory order option?
There was a problem hiding this comment.
It's used for synchronization, where are some while(!get_functions_txt_parsed()) loops. I'm not sure that it isn't synchronized with other atomic variables.
mkornaukhov
assigned
PetrShumilov and
mkornaukhov and unassigned
mkornaukhov and
PetrShumilov
2 participants
I have tried to run transpiler with TSAN on simple program and found lots of data races. This PR fixes some of them, but not all.