[PATCH] D47020: [Windows, Process] Fix an issue in windows thread handling that was causing LLDB to hang

[Stella Stamenova via Phabricator via llvm-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL332670: [Windows, Process] Fix an issue in windows thread handling that was causing… (authored by stella.stamenova, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D47020?vs=147390&id=147393#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D47020

Files:
  lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
  lldb/trunk/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp
  lldb/trunk/source/Plugins/Process/Windows/Common/TargetThreadWindows.h


Index: lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
===================================================================
--- lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
+++ lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
@@ -349,13 +349,23 @@
 
     LLDB_LOG(log, "resuming {0} threads.", m_thread_list.GetSize());
 
+    bool failed = false;
     for (uint32_t i = 0; i < m_thread_list.GetSize(); ++i) {
       auto thread = std::static_pointer_cast<TargetThreadWindows>(
           m_thread_list.GetThreadAtIndex(i));
-      thread->DoResume();
+      Status result = thread->DoResume();
+      if (result.Fail()) {
+        failed = true;
+        LLDB_LOG(log, "Trying to resume thread at index {0}, but failed with error {1}.", i, result);
+      }
     }
 
-    SetPrivateState(eStateRunning);
+    if (failed) {
+      error.SetErrorString("ProcessWindows::DoResume failed");
+      return error;
+    } else {
+      SetPrivateState(eStateRunning);
+    }
   } else {
     LLDB_LOG(log, "error: process %I64u is in state %u.  Returning...",
              m_session_data->m_debugger->GetProcess().GetProcessId(),
Index: lldb/trunk/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp
===================================================================
--- lldb/trunk/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp
+++ lldb/trunk/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp
@@ -98,11 +98,11 @@
   return m_unwinder_ap.get();
 }
 
-bool TargetThreadWindows::DoResume() {
+Status TargetThreadWindows::DoResume() {
   StateType resume_state = GetTemporaryResumeState();
   StateType current_state = GetState();
   if (resume_state == current_state)
-    return true;
+    return Status();
 
   if (resume_state == eStateStepping) {
     uint32_t flags_index =
@@ -118,8 +118,16 @@
     DWORD previous_suspend_count = 0;
     HANDLE thread_handle = m_host_thread.GetNativeThread().GetSystemHandle();
     do {
+      // ResumeThread returns -1 on error, or the thread's *previous* suspend count on success.
+      // This means that the return value is 1 when the thread was restarted.
+      // Note that DWORD is an unsigned int, so we need to explicitly compare with -1.
       previous_suspend_count = ::ResumeThread(thread_handle);
-    } while (previous_suspend_count > 0);
+
+      if (previous_suspend_count == (DWORD)-1)
+        return Status(::GetLastError(), eErrorTypeWin32);
+
+    } while (previous_suspend_count > 1);
   }
-  return true;
+
+  return Status();
 }
Index: lldb/trunk/source/Plugins/Process/Windows/Common/TargetThreadWindows.h
===================================================================
--- lldb/trunk/source/Plugins/Process/Windows/Common/TargetThreadWindows.h
+++ lldb/trunk/source/Plugins/Process/Windows/Common/TargetThreadWindows.h
@@ -37,7 +37,7 @@
   bool CalculateStopInfo() override;
   Unwind *GetUnwinder() override;
 
-  bool DoResume();
+  Status DoResume();
 
   HostThread GetHostThread() const { return m_host_thread; }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D47020.147393.patch
Type: text/x-patch
Size: 3087 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180517/64a529a3/attachment.bin>