[Lldb-commits] [lldb] [lldb][windows] fix a race condition in IO reader thread (PR #194422)

[via lldb-commits]

================
@@ -740,9 +740,39 @@ ProcessWindows::OnDebugException(bool first_chance,
     return ExceptionResult::SendToApplication;
   }
 
+  // Drain any in-flight process output before announcing the stop. The I/O
+  // reader thread and this debug-event thread run concurrently. Without
+  // synchronization the eBroadcastBitStateChanged(Stopped) event can reach
+  // the Debugger event thread before the preceding eBroadcastBitSTDOUT
+  // events.
+  auto drain_stdout = [this] {
+    if (!m_stdio_communication.ReadThreadIsRunning())
+      return;
+    m_stdio_communication.SynchronizeWithReadThread();
+    if (!m_pty || m_pty->GetMode() != PseudoConsole::Mode::ConPTY)
+      return;
+
+    HANDLE pipe = m_pty->GetSTDOUTHandle();
+    for (int consec_empty = 0; consec_empty < 3;) {
+      DWORD avail = 0;
+      if (!::PeekNamedPipe(pipe, nullptr, 0, nullptr, &avail, nullptr))
+        break;
+      if (avail > 0) {
+        consec_empty = 0;
+        if (m_stdio_communication.ReadThreadIsRunning())
+          m_stdio_communication.SynchronizeWithReadThread();
----------------
Nerixyz wrote:

> We can infer that it's making progress if there is data in the pipe with `PeekNamedPipe`. If there is data, the thread should read it.

I see, so because we communicate with the thread, we know it has to make progress.

> The issue I see with this is "what is blocking" here? Does it mean reaching the 5s timeout? We are implicitely doing something similar here: "if we don't see anything in the pipe after peeking 3 consecutive times every 1ms, we assume the conpty is done".

With no blocking I meant a `WaitForMultipleObjects(timeout=0)` to check if there's something. But I think that has problems too.

https://github.com/llvm/llvm-project/pull/194422