[Lldb-commits] [lldb] [lldb][windows] fix a race condition when polling Pipes (PR #172063)

[Charles Zablit via lldb-commits]

https://github.com/charles-zablit created https://github.com/llvm/llvm-project/pull/172063

This patch fixes a deadlock due to a race condition inside the `PipeEvent` class.

The lldp-dap tests on Windows are very prone to this deadlock (from my testing almost 1/2 times `TestDAP_Launch.py` fails because of a deadlock inside `PipeEvent`.

Before this patch, the destructor could try to set `m_ready` right before `PipeEvent::Monitor` would reset it, causing `WaitForSingleObject(m_ready, INFINITE);` to deadlock.

This is fixed by ensuring all the `SetEvent` and `ResetEvent` calls inside the class are behind a mutex.

>From 55275e2f97752ab37ac4d67c011f93a2b3cbfe88 Mon Sep 17 00:00:00 2001
From: Charles Zablit <c_zablit at apple.com>
Date: Fri, 12 Dec 2025 18:28:39 +0000
Subject: [PATCH] [lldb][windows] fix a race condition when polling Pipes

---
 lldb/source/Host/windows/MainLoopWindows.cpp | 61 +++++++++++++-------
 1 file changed, 39 insertions(+), 22 deletions(-)

diff --git a/lldb/source/Host/windows/MainLoopWindows.cpp b/lldb/source/Host/windows/MainLoopWindows.cpp
index 5e5888aee2181..a22a8f002a2ee 100644
--- a/lldb/source/Host/windows/MainLoopWindows.cpp
+++ b/lldb/source/Host/windows/MainLoopWindows.cpp
@@ -53,9 +53,12 @@ class PipeEvent : public MainLoopWindows::IOEvent {
 
   ~PipeEvent() override {
     if (m_monitor_thread.joinable()) {
-      m_stopped = true;
-      SetEvent(m_ready);
-      CancelIoEx(m_handle, /*lpOverlapped=*/NULL);
+      {
+        std::lock_guard<std::mutex> guard(m_mutex);
+        m_stopped = true;
+        SetEvent(m_ready);
+        CancelIoEx(m_handle, &m_ov);
+      }
       m_monitor_thread.join();
     }
     CloseHandle(m_event);
@@ -63,20 +66,25 @@ class PipeEvent : public MainLoopWindows::IOEvent {
   }
 
   void WillPoll() override {
-    if (WaitForSingleObject(m_event, /*dwMilliseconds=*/0) != WAIT_TIMEOUT) {
-      // The thread has already signalled that the data is available. No need
-      // for further polling until we consume that event.
-      return;
-    }
-    if (WaitForSingleObject(m_ready, /*dwMilliseconds=*/0) != WAIT_TIMEOUT) {
-      // The thread is already waiting for data to become available.
+    std::lock_guard<std::mutex> guard(m_mutex);
+
+    HANDLE handles[2] = {m_event, m_ready};
+    if (WaitForMultipleObjects(2, handles, /*bWaitAll=*/FALSE,
+                               /*dwMilliseconds=*/0) != WAIT_TIMEOUT) {
+      // Either:
+      // - The thread has already signalled that the data is available. No need
+      //   for further polling until we consume that event.
+      // - The thread is already waiting for data to become available.
       return;
     }
     // Start waiting.
     SetEvent(m_ready);
   }
 
-  void Disarm() override { ResetEvent(m_event); }
+  void Disarm() override {
+    std::lock_guard<std::mutex> guard(m_mutex);
+    ResetEvent(m_event);
+  }
 
   /// Monitors the handle performing a zero byte read to determine when data is
   /// avaiable.
@@ -87,17 +95,16 @@ class PipeEvent : public MainLoopWindows::IOEvent {
     do {
       char buf[1];
       DWORD bytes_read = 0;
-      OVERLAPPED ov;
-      ZeroMemory(&ov, sizeof(ov));
+      ZeroMemory(&m_ov, sizeof(m_ov));
       // Block on a 0-byte read; this will only resume when data is
       // available in the pipe. The pipe must be PIPE_WAIT or this thread
       // will spin.
-      BOOL success =
-          ReadFile(m_handle, buf, /*nNumberOfBytesToRead=*/0, &bytes_read, &ov);
+      BOOL success = ReadFile(m_handle, buf, /*nNumberOfBytesToRead=*/0,
+                              &bytes_read, &m_ov);
       DWORD bytes_available = 0;
       DWORD err = GetLastError();
       if (!success && err == ERROR_IO_PENDING) {
-        success = GetOverlappedResult(m_handle, &ov, &bytes_read,
+        success = GetOverlappedResult(m_handle, &m_ov, &bytes_read,
                                       /*bWait=*/TRUE);
         err = GetLastError();
       }
@@ -119,12 +126,20 @@ class PipeEvent : public MainLoopWindows::IOEvent {
         // Read may have been cancelled, try again.
         continue;
       }
-
-      // Notify that data is available on the pipe. It's important to set this
-      // before clearing m_ready to avoid a race with WillPoll.
-      SetEvent(m_event);
-      // Stop polling until we're told to resume.
-      ResetEvent(m_ready);
+      {
+        std::lock_guard<std::mutex> guard(m_mutex);
+
+        // Notify that data is available on the pipe.
+        SetEvent(m_event);
+        if (m_stopped) {
+          // The destructor might have called SetEvent(m_ready) before this
+          // block. It that's the case, ResetEvent(m_ready) will cause
+          // WaitForSingleObject to wait forever unless we break early.
+          break;
+        }
+        // Stop polling until we're told to resume.
+        ResetEvent(m_ready);
+      }
 
       // Wait until the current read is consumed before doing the next read.
       WaitForSingleObject(m_ready, INFINITE);
@@ -134,8 +149,10 @@ class PipeEvent : public MainLoopWindows::IOEvent {
 private:
   HANDLE m_handle;
   HANDLE m_ready;
+  OVERLAPPED m_ov;
   std::thread m_monitor_thread;
   std::atomic<bool> m_stopped = false;
+  std::mutex m_mutex;
 };
 
 class SocketEvent : public MainLoopWindows::IOEvent {