augusto2112 added inline comments.
================
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);
----------------
bulbazord wrote:
> 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).
You're right, nice catch.
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
