augusto2112 added inline comments.
================
Comment at: lldb/include/lldb/Core/Communication.h:86-89
+ lldb_private::Connection *GetConnection() {
+ std::shared_lock guard(m_connection_mutex);
+ return m_connection_sp.get();
+ }
----------------
bulbazord wrote:
> This isn't necessarily introduced by your patch, but I don't think this lock
> does much other than make sure nobody can change `m_connection_sp` in between
> you calling `GetConnection` and you actually receiving the `Connection`
> pointer. It still comes with all the pitfalls of handing out a raw pointer
> (instead of a `shared_ptr`).
>
> As a follow-up or maybe even precursor, we should probably stop handing out
> the raw connection pointer...
I agree. I think we should do it after this patch though.
================
Comment at: lldb/include/lldb/Core/Communication.h:172-174
std::mutex
m_write_mutex; ///< Don't let multiple threads write at the same time...
+ mutable std::shared_mutex m_connection_mutex;
----------------
JDevlieghere wrote:
> Why do we need both? Can't we use `m_connection_mutex` in write mode instead
> of `m_write_mutex`?
If we use `m_connection_mutex` in write mode instead of m_write_mutex then we
can't have concurrent reads and writes.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157760/new/
https://reviews.llvm.org/D157760
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
