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

[Charles Zablit 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();
----------------
charles-zablit wrote:

> If the read-thread isn't running anymore, we need to exit the loop.

Fixed, thanks.

> We can't really guarantee that the read-thread makes progress, right?

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.

> Could we somehow tell it to "read until it would block"?

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".

I think the fundamental issue with the ConPTY overall is that we can't know for sure when all the data has arrived. We can only make a best effort.

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