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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
