[Lldb-commits] [lldb] [lldb][windows] fix race condition in ConPTY on process exit (PR #194631)

[via lldb-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Charles Zablit (charles-zablit)

<details>
<summary>Changes</summary>

Two changes to fix a race condition where the ConPTY read thread could block for 5 seconds on process exit:

**`ConnectionConPTYWindows.cpp`:** After the read thread wakes from the CV (signalled by `PseudoConsole::Close`), check `IsConnected()` before entering `ConnectionGenericFile::Read`. If the ConPTY has been closed, return `eConnectionStatusEndOfFile` immediately instead of calling `ReadFile` that would block until the 5-second timeout.

**`ProcessWindows.cpp`:** In `OnExitProcess`, move `InterruptRead` after `Close` (so the interrupt isn't consumed while the read thread is still waiting on the CV), and call `StopReadThread` to join the read thread before announcing the exit. This ensures the read thread is fully stopped before `SetPrivateState(eStateExited)` broadcasts.

I ran debugged a simple program at desk, in a loop for 2500 iterations. Before this change, the race would happen in the first 50 iterations. Now it does not happen after 2500 iterations.

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


2 Files Affected:

- (modified) lldb/source/Host/windows/ConnectionConPTYWindows.cpp (+6-1) 
- (modified) lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp (+2-1) 


``````````diff
diff --git a/lldb/source/Host/windows/ConnectionConPTYWindows.cpp b/lldb/source/Host/windows/ConnectionConPTYWindows.cpp
index 39ca3522cb947..c49aae7b3104e 100644
--- a/lldb/source/Host/windows/ConnectionConPTYWindows.cpp
+++ b/lldb/source/Host/windows/ConnectionConPTYWindows.cpp
@@ -65,8 +65,13 @@ size_t ConnectionConPTY::Read(void *dst, size_t dst_len,
                               lldb::ConnectionStatus &status,
                               Status *error_ptr) {
   std::unique_lock<std::mutex> guard(m_pty->GetMutex());
-  if (m_pty->IsStopping()) {
+  if (m_pty->IsStopping())
     m_pty->GetCV().wait(guard, [this] { return !m_pty->IsStopping(); });
+  guard.unlock();
+
+  if (!m_pty->IsConnected()) {
+    status = eConnectionStatusEndOfFile;
+    return 0;
   }
 
   size_t bytes_read =
diff --git a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
index c0c8be8bd62b8..e5c4ed991e3bf 100644
--- a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
+++ b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
@@ -656,8 +656,9 @@ void ProcessWindows::OnExitProcess(uint32_t exit_code) {
 
   if (m_pty) {
     m_pty->SetStopping(true);
-    m_stdio_communication.InterruptRead();
     m_pty->Close();
+    m_stdio_communication.InterruptRead();
+    m_stdio_communication.StopReadThread();
   }
 
   TargetSP target = CalculateTarget();

``````````

</details>


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