[lldb/windows] Reset MainLoop events after handling them

[labath]

This prevents the callback function from being called in a busy loop. Discovered by @slydiman on #106955.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Pavel Labath (labath)

Changes

This prevents the callback function from being called in a busy loop. Discovered by @slydiman on #106955.

---

Full diff: https://github.com/llvm/llvm-project/pull/107061.diff

2 Files Affected:

• (modified) lldb/source/Host/windows/MainLoopWindows.cpp (+1)
• (modified) lldb/unittests/Host/MainLoopTest.cpp (+39)

diff --git a/lldb/source/Host/windows/MainLoopWindows.cpp b/lldb/source/Host/windows/MainLoopWindows.cpp
index d34972a93f5850..551e73e6904ae7 100644
--- a/lldb/source/Host/windows/MainLoopWindows.cpp
+++ b/lldb/source/Host/windows/MainLoopWindows.cpp
@@ -125,6 +125,7 @@ Status MainLoopWindows::Run() {
 
     if (*signaled_event < m_read_fds.size()) {
       auto &KV = *std::next(m_read_fds.begin(), *signaled_event);
+      WSAResetEvent(KV.second.event);
       ProcessReadObject(KV.first);
     } else {
       assert(*signaled_event == m_read_fds.size());
diff --git a/lldb/unittests/Host/MainLoopTest.cpp b/lldb/unittests/Host/MainLoopTest.cpp
index 4084e90782fd5d..5843faeab505ee 100644
--- a/lldb/unittests/Host/MainLoopTest.cpp
+++ b/lldb/unittests/Host/MainLoopTest.cpp
@@ -15,6 +15,7 @@
 #include "llvm/Testing/Support/Error.h"
 #include "gtest/gtest.h"
 #include <future>
+#include <thread>
 
 using namespace lldb_private;
 
@@ -78,6 +79,44 @@ TEST_F(MainLoopTest, ReadObject) {
   ASSERT_EQ(1u, callback_count);
 }
 
+TEST_F(MainLoopTest, NoSpuriousReads) {
+  // Write one byte into the socket.
+  char X = 'X';
+  size_t len = sizeof(X);
+  ASSERT_TRUE(socketpair[0]->Write(&X, len).Success());
+
+  MainLoop loop;
+
+  Status error;
+  auto handle = loop.RegisterReadObject(
+      socketpair[1],
+      [this](MainLoopBase &) {
+        if (callback_count == 0) {
+          // Read the byte back the first time we're called. After that, the
+          // socket is empty, and we should not be called anymore.
+          char X;
+          size_t len = sizeof(X);
+          EXPECT_THAT_ERROR(socketpair[1]->Read(&X, len).ToError(),
+                            llvm::Succeeded());
+          EXPECT_EQ(len, sizeof(X));
+        }
+        ++callback_count;
+      },
+      error);
+  ASSERT_THAT_ERROR(error.ToError(), llvm::Succeeded());
+  // Terminate the loop after one second.
+  std::thread terminate_thread([&loop] {
+    std::this_thread::sleep_for(std::chrono::seconds(1));
+    loop.AddPendingCallback(
+        [](MainLoopBase &loop) { loop.RequestTermination(); });
+  });
+  ASSERT_THAT_ERROR(loop.Run().ToError(), llvm::Succeeded());
+  terminate_thread.join();
+
+  // Make sure the callback was called only once.
+  ASSERT_EQ(1u, callback_count);
+}
+
 TEST_F(MainLoopTest, TerminatesImmediately) {
   char X = 'X';
   size_t len = sizeof(X);

[DavidSpickett]

Is this related to #106283 as well?

[labath]

Yeah, that's probably it.

[slydiman]

LGTM, thanks