[Lldb-commits] [PATCH] D135516: [lldb] [MainLoopPosix] Fix crash upon adding lots of pending callbacks

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 467834.
mgorny added a comment.

Move and rename the `bool`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135516/new/

https://reviews.llvm.org/D135516

Files:
  lldb/include/lldb/Host/posix/MainLoopPosix.h
  lldb/source/Host/posix/MainLoopPosix.cpp
  lldb/unittests/Host/MainLoopTest.cpp


Index: lldb/unittests/Host/MainLoopTest.cpp
===================================================================
--- lldb/unittests/Host/MainLoopTest.cpp
+++ lldb/unittests/Host/MainLoopTest.cpp
@@ -171,6 +171,17 @@
   ASSERT_TRUE(callback2_called);
 }
 
+// Regression test for assertion failure if a lot of callbacks end up
+// being queued after loop exits.
+TEST_F(MainLoopTest, PendingCallbackAfterLoopExited) {
+  MainLoop loop;
+  Status error;
+  ASSERT_TRUE(loop.Run().Success());
+  // Try to fill the pipe buffer in.
+  for (int i = 0; i < 65536; ++i)
+    loop.AddPendingCallback([&](MainLoopBase &loop) {});
+}
+
 #ifdef LLVM_ON_UNIX
 TEST_F(MainLoopTest, DetectsEOF) {
 
Index: lldb/source/Host/posix/MainLoopPosix.cpp
===================================================================
--- lldb/source/Host/posix/MainLoopPosix.cpp
+++ lldb/source/Host/posix/MainLoopPosix.cpp
@@ -222,7 +222,7 @@
 }
 #endif
 
-MainLoopPosix::MainLoopPosix() {
+MainLoopPosix::MainLoopPosix() : m_triggering(false) {
   Status error = m_trigger_pipe.CreateNew(/*child_process_inherit=*/false);
   assert(error.Success());
   const int trigger_pipe_fd = m_trigger_pipe.GetReadFileDescriptor();
@@ -371,6 +371,7 @@
 
     impl.ProcessEvents();
 
+    m_triggering = false;
     ProcessPendingCallbacks();
   }
   return Status();
@@ -395,6 +396,10 @@
 }
 
 void MainLoopPosix::TriggerPendingCallbacks() {
+  if (m_triggering)
+    return;
+  m_triggering = true;
+
   char c = '.';
   size_t bytes_written;
   Status error = m_trigger_pipe.Write(&c, 1, bytes_written);
Index: lldb/include/lldb/Host/posix/MainLoopPosix.h
===================================================================
--- lldb/include/lldb/Host/posix/MainLoopPosix.h
+++ lldb/include/lldb/Host/posix/MainLoopPosix.h
@@ -13,6 +13,7 @@
 #include "lldb/Host/MainLoopBase.h"
 #include "lldb/Host/Pipe.h"
 #include "llvm/ADT/DenseMap.h"
+#include <atomic>
 #include <csignal>
 #include <list>
 #include <vector>
@@ -87,6 +88,7 @@
   llvm::DenseMap<IOObject::WaitableHandle, Callback> m_read_fds;
   llvm::DenseMap<int, SignalInfo> m_signals;
   Pipe m_trigger_pipe;
+  std::atomic<bool> m_triggering;
 #if HAVE_SYS_EVENT_H
   int m_kqueue;
 #endif



Index: lldb/unittests/Host/MainLoopTest.cpp
===================================================================
--- lldb/unittests/Host/MainLoopTest.cpp
+++ lldb/unittests/Host/MainLoopTest.cpp
@@ -171,6 +171,17 @@
   ASSERT_TRUE(callback2_called);
 }
 
+// Regression test for assertion failure if a lot of callbacks end up
+// being queued after loop exits.
+TEST_F(MainLoopTest, PendingCallbackAfterLoopExited) {
+  MainLoop loop;
+  Status error;
+  ASSERT_TRUE(loop.Run().Success());
+  // Try to fill the pipe buffer in.
+  for (int i = 0; i < 65536; ++i)
+    loop.AddPendingCallback([&](MainLoopBase &loop) {});
+}
+
 #ifdef LLVM_ON_UNIX
 TEST_F(MainLoopTest, DetectsEOF) {
 
Index: lldb/source/Host/posix/MainLoopPosix.cpp
===================================================================
--- lldb/source/Host/posix/MainLoopPosix.cpp
+++ lldb/source/Host/posix/MainLoopPosix.cpp
@@ -222,7 +222,7 @@
 }
 #endif
 
-MainLoopPosix::MainLoopPosix() {
+MainLoopPosix::MainLoopPosix() : m_triggering(false) {
   Status error = m_trigger_pipe.CreateNew(/*child_process_inherit=*/false);
   assert(error.Success());
   const int trigger_pipe_fd = m_trigger_pipe.GetReadFileDescriptor();
@@ -371,6 +371,7 @@
 
     impl.ProcessEvents();
 
+    m_triggering = false;
     ProcessPendingCallbacks();
   }
   return Status();
@@ -395,6 +396,10 @@
 }
 
 void MainLoopPosix::TriggerPendingCallbacks() {
+  if (m_triggering)
+    return;
+  m_triggering = true;
+
   char c = '.';
   size_t bytes_written;
   Status error = m_trigger_pipe.Write(&c, 1, bytes_written);
Index: lldb/include/lldb/Host/posix/MainLoopPosix.h
===================================================================
--- lldb/include/lldb/Host/posix/MainLoopPosix.h
+++ lldb/include/lldb/Host/posix/MainLoopPosix.h
@@ -13,6 +13,7 @@
 #include "lldb/Host/MainLoopBase.h"
 #include "lldb/Host/Pipe.h"
 #include "llvm/ADT/DenseMap.h"
+#include <atomic>
 #include <csignal>
 #include <list>
 #include <vector>
@@ -87,6 +88,7 @@
   llvm::DenseMap<IOObject::WaitableHandle, Callback> m_read_fds;
   llvm::DenseMap<int, SignalInfo> m_signals;
   Pipe m_trigger_pipe;
+  std::atomic<bool> m_triggering;
 #if HAVE_SYS_EVENT_H
   int m_kqueue;
 #endif

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits