JDevlieghere 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);
----------------
augusto2112 wrote:
> 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.
Thanks, I didn't know about that and that was certainly not the intention.
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
