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

[Charles Zablit via lldb-commits]

================
@@ -118,17 +65,21 @@ 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()) {
----------------
charles-zablit wrote:

> Don't we need the guard to check for `IsConnected()`?

I was going to say that:
- If `m_pty->IsStopping()` is `true`, by the time we reach `m_pty->IsConnected()`, the conpty is guaranteed to be closed.
- If it's false, the conpty is not stopping, the IsConnected check can run race free.

But that's not true in the second case, there is still a race window. I added the IsConnected check to the critical section.

I did not second guess my change too much after it worked :)

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