clayborg added inline comments.
================ Comment at: lldb/include/lldb/Utility/Log.h:54 + virtual void Flush() = 0; + std::mutex &GetMutex() { return m_mutex; } + ---------------- Maybe we don't expose this mutex at all and we have a EmitThreadSafe(...) method?: ``` void EmitThreadSafe(llvm::StringRef message) { std::lock_guard<std::mutex> guard(m_mutex); Emit(message); } ``` ================ Comment at: lldb/source/Utility/Log.cpp:323-324 if (options.Test(LLDB_LOG_OPTION_THREADSAFE)) { - static std::recursive_mutex g_LogThreadedMutex; - std::lock_guard<std::recursive_mutex> guard(g_LogThreadedMutex); - *stream_sp << message; - stream_sp->flush(); + std::lock_guard<std::mutex> guard(handler_sp->GetMutex()); + handler_sp->Emit(message); + handler_sp->Flush(); ---------------- If we like the idea of EmitThreadSafe then the code can look like this ================ Comment at: lldb/source/Utility/Log.cpp:325 + handler_sp->Emit(message); + handler_sp->Flush(); } else { ---------------- Do we ever not call Flush() after a Emit() call? If so do we even need the flush call, or should we just leave it up to the specific implementation to do what it wants in Emit(...)? ================ Comment at: lldb/source/Utility/Log.cpp:328 + handler_sp->Emit(message); + handler_sp->Flush(); } ---------------- Do we ever not call Flush() after a Emit() call? If so do we even need the flush call, or should we just leave it up to the specific implementation to do what it wants in Emit(...)? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127922/new/ https://reviews.llvm.org/D127922 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits