[Lldb-commits] [PATCH] D120762: [lldb] Avoid data race in IOHandlerProcessSTDIO

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

How is using m_mutex better than just using the std::atomic<bool>? Just protecting the modification of the value with a mutex doesn't seem like it would do much more than the atomic already did unless we are using the lock the protect the value for longer that just the modification.



================
Comment at: lldb/source/Target/Process.cpp:4384
   void Cancel() override {
+    std::lock_guard<std::mutex> guard(m_mutex);
     SetIsDone(true);
----------------
Won't this cause a deadlock? You aren't using a recursive mutex here and you are locking the "m_mutex" and then calling SetIsDone(true) which will try and lock the mutex as well?


================
Comment at: lldb/source/Target/Process.cpp:4441
   Pipe m_pipe;
-  std::atomic<bool> m_is_running{false};
+  std::mutex m_mutex;
+  bool m_is_running = false;
----------------
Does this need to be a recursive mutex? We have the same thread locking this mutex multiple times it seems from the code?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120762/new/

https://reviews.llvm.org/D120762