[Lldb-commits] [PATCH] D38938: Logging: provide a way to safely disable logging in a forked process

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.

We had a bug where if we had forked (in the ProcessLauncherPosixFork)
while another thread was writing a log message, we would deadlock. This
happened because the fork child inherited the locked log rwmutex, which
would never get unlocked. This meant the child got stuck trying to
disable all log channels.

The bug existed for a while but only started being apparent after
https://reviews.llvm.org/D37930, which started using ThreadLauncher (which uses logging) instead
of std::thread (which does not) for launching TaskPool threads.

The fix is to provide a lock free way to disable logging, which should
be used in this case, and this case only.


https://reviews.llvm.org/D38938

Files:
  include/lldb/Utility/Log.h
  source/Host/posix/ProcessLauncherPosixFork.cpp
  source/Utility/Log.cpp


Index: source/Utility/Log.cpp
===================================================================
--- source/Utility/Log.cpp
+++ source/Utility/Log.cpp
@@ -90,7 +90,10 @@
 
 void Log::Disable(uint32_t flags) {
   llvm::sys::ScopedWriter lock(m_mutex);
+  DisableUnlocked(flags);
+}
 
+void Log::DisableUnlocked(uint32_t flags) {
   uint32_t mask = m_mask.fetch_and(~flags, std::memory_order_relaxed);
   if (!(mask & ~flags)) {
     m_stream_sp.reset();
@@ -241,6 +244,11 @@
     entry.second.Disable(UINT32_MAX);
 }
 
+void Log::DisableAllLogChannelsUnlocked() {
+  for (auto &entry : *g_channel_map)
+    entry.second.DisableUnlocked(UINT32_MAX);
+}
+
 void Log::ListAllLogChannels(llvm::raw_ostream &stream) {
   if (g_channel_map->empty()) {
     stream << "No logging channels are currently registered.\n";
Index: source/Host/posix/ProcessLauncherPosixFork.cpp
===================================================================
--- source/Host/posix/ProcessLauncherPosixFork.cpp
+++ source/Host/posix/ProcessLauncherPosixFork.cpp
@@ -97,7 +97,7 @@
                                               const ProcessLaunchInfo &info) {
   // First, make sure we disable all logging. If we are logging to stdout, our
   // logs can be mistaken for inferior output.
-  Log::DisableAllLogChannels();
+  Log::DisableAllLogChannelsUnlocked();
 
   // Do not inherit setgid powers.
   if (setgid(getgid()) != 0)
Index: include/lldb/Utility/Log.h
===================================================================
--- include/lldb/Utility/Log.h
+++ include/lldb/Utility/Log.h
@@ -116,6 +116,11 @@
 
   static void DisableAllLogChannels();
 
+  // Only use if no other threads are executing and calling the locking version
+  // is known to be unsafe (for example, this happens right after forking, if we
+  // have forked while another thread was holding the log mutex)).
+  static void DisableAllLogChannelsUnlocked();
+
   static void ListAllLogChannels(llvm::raw_ostream &stream);
 
   //------------------------------------------------------------------
@@ -184,6 +189,7 @@
               uint32_t options, uint32_t flags);
 
   void Disable(uint32_t flags);
+  void DisableUnlocked(uint32_t flags);
 
   typedef llvm::StringMap<Log> ChannelMap;
   static llvm::ManagedStatic<ChannelMap> g_channel_map;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D38938.119098.patch
Type: text/x-patch
Size: 2299 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20171015/ae152e43/attachment.bin>