[Lldb-commits] [PATCH] D30702: Fix remaining threading issues in Log.h

[Zachary Turner via Phabricator via lldb-commits]

zturner added inline comments.


================
Comment at: source/Utility/Log.cpp:90-91
                           uint32_t options, uint32_t flags) {
-  log.GetMask().Set(flags);
-  if (log.GetMask().Get()) {
-    log.GetOptions().Reset(options);
+  uint32_t mask = log.m_mask.load(std::memory_order_acquire) | flags;
+  log.m_mask.store(mask, std::memory_order_release);
+  if (mask) {
----------------
How about 

```
uint32_t old_mask = log.m_mask.fetch_or(flags);
if (old_mask  & flags) {
  ...
}
```

Also, there is still a race if two people call this function at the same time.


================
Comment at: source/Utility/Log.cpp:103-104
     return;
-  log->GetMask().Clear(flags);
-  if (!log->GetMask().Get()) {
+  uint32_t mask = log->m_mask.load(std::memory_order_acquire) & ~flags;
+  log->m_mask.store(mask, std::memory_order_release);
+  if (!mask) {
----------------
```
uint32_t old_mask = log->m_mask.fetch_and(~flags);
if (!(old_mask & ~flags)) {
  ...
}
```


================
Comment at: source/Utility/Log.cpp:116
+const Flags Log::GetMask() const {
+  return m_mask.load(std::memory_order_acquire);
+}
----------------
Any reason you always use the explicit `load(memory_order_acquire)` syntax instead of using the default sequentially consistent ordering?


https://reviews.llvm.org/D30702