Author: Michał Górny Date: 2022-10-17T17:48:44+02:00 New Revision: e8ee0f121dbc2dc20a9de326531c4d4d061a20d8
URL: https://github.com/llvm/llvm-project/commit/e8ee0f121dbc2dc20a9de326531c4d4d061a20d8 DIFF: https://github.com/llvm/llvm-project/commit/e8ee0f121dbc2dc20a9de326531c4d4d061a20d8.diff LOG: [lldb] [MainLoopPosix] Fix crash upon adding lots of pending callbacks If lots of pending callbacks are added while the main loop has exited already, the trigger pipe buffer fills in, causing the write to fail and the related assertion to fail. To avoid this, add a boolean member indicating whether the callbacks have been triggered already. If the trigger was done, avoid writing to the pipe until loops proceeds to run them and resets the variable. Besides fixing the issue, this also avoids writing to the pipe multiple times if callbacks are added faster than the loop is able to process them. Previously, this would lead to the loop performing multiple read iterations from pipe unnecessarily. Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.llvm.org/D135516 Added: Modified: lldb/include/lldb/Host/posix/MainLoopPosix.h lldb/source/Host/posix/MainLoopPosix.cpp lldb/unittests/Host/MainLoopTest.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/Host/posix/MainLoopPosix.h b/lldb/include/lldb/Host/posix/MainLoopPosix.h index 18185c3c5ee78..07497b7b8c259 100644 --- a/lldb/include/lldb/Host/posix/MainLoopPosix.h +++ b/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 @@ class MainLoopPosix : public MainLoopBase { 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 diff --git a/lldb/source/Host/posix/MainLoopPosix.cpp b/lldb/source/Host/posix/MainLoopPosix.cpp index 37d339d8bc779..b185c3d3b7076 100644 --- a/lldb/source/Host/posix/MainLoopPosix.cpp +++ b/lldb/source/Host/posix/MainLoopPosix.cpp @@ -222,7 +222,7 @@ void MainLoopPosix::RunImpl::ProcessEvents() { } #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 @@ Status MainLoopPosix::Run() { impl.ProcessEvents(); + m_triggering = false; ProcessPendingCallbacks(); } return Status(); @@ -395,6 +396,9 @@ void MainLoopPosix::ProcessSignal(int signo) { } void MainLoopPosix::TriggerPendingCallbacks() { + if (m_triggering.exchange(true)) + return; + char c = '.'; size_t bytes_written; Status error = m_trigger_pipe.Write(&c, 1, bytes_written); diff --git a/lldb/unittests/Host/MainLoopTest.cpp b/lldb/unittests/Host/MainLoopTest.cpp index 00e85514463fe..b1857fec3c49e 100644 --- a/lldb/unittests/Host/MainLoopTest.cpp +++ b/lldb/unittests/Host/MainLoopTest.cpp @@ -171,6 +171,17 @@ TEST_F(MainLoopTest, PendingCallbackTrigger) { 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) { _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
