[Lldb-commits] [PATCH] D157347: [lldb] Fix data race in NativeFile

[Alex Langford via Phabricator via lldb-commits]

bulbazord requested changes to this revision.
bulbazord added inline comments.
This revision now requires changes to proceed.


================
Comment at: lldb/source/Host/common/File.cpp:609
       num_bytes = bytes_written;
-  } else if (StreamIsValid()) {
+  } else if (ValueGuard stream_guard = StreamIsValid()) {
     bytes_written = ::fwrite(buf, 1, num_bytes, m_stream);
----------------
augusto2112 wrote:
> One thing that's not super obvious for people reading this is that `descriptor_guard` is still in scope in the else branch (due to C++'s weird scoping rules when it comes to declaring variables inside `if` statements), and will keep the descriptor mutex locked which is probably not what you intended. I don't think this affects the correctness of this implementation at the moment, but could be dangerous with further changes on top of this one.
If that's true, then this change needs further adjustment. `GetStream` acquires the `ValueGuard`s in the opposite order, meaning Thread 1 can call this function and acquire `descriptor_guard` while Thread 2 can call `GetStream` and acquire `stream_guard` at the same time. Then they're both waiting on the other lock (deadlock).


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

https://reviews.llvm.org/D157347