[Lldb-commits] [PATCH] D127922: [lldb] Introduce the concept of a log handler (NFC)

[Greg Clayton via Phabricator via lldb-commits]

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