[Lldb-commits] [lldb] [lldb] Fix TestThreadExit.py flakiness (PR #190976)

[via lldb-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

<details>
<summary>Changes</summary>

When `pthread_join` returns, the target thread signals its internal semaphore, but the underlying Mach thread hasn't been removed from the task yet with `thread_terminate`.

The flakiness is the result of the debugger stopping the process halts the dying thread mid-termination. There is no Mach API to distinguish a dying thread from a live one: it appears as `TH_STATE_STOPPED`, like any other suspended thread.

This PR adds a helper (`wait_for_thread_cleanup`) that polls `task_threads` to ensure the Mach thread is actually gone before the breakpoint. Since there might be other tests that are affected by this race, I put it in a common location so it can be reused.

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


2 Files Affected:

- (added) lldb/packages/Python/lldbsuite/test/make/mach_thread.h (+26) 
- (modified) lldb/test/API/functionalities/thread/thread_exit/main.cpp (+37-39) 


``````````diff
diff --git a/lldb/packages/Python/lldbsuite/test/make/mach_thread.h b/lldb/packages/Python/lldbsuite/test/make/mach_thread.h
new file mode 100644
index 0000000000000..3c00811a6e79c
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/make/mach_thread.h
@@ -0,0 +1,26 @@
+#ifdef __APPLE__
+#include <mach/mach.h>
+#include <thread>
+#endif
+
+// After join returns, the underlying Mach thread may not have been terminated
+// yet (pthread_join uses a semaphore that is signaled before the thread calls
+// thread_terminate). If the debugger stops the process in this window it will
+// freeze the dying thread, making it appear still alive in task_threads(). Poll
+// until the kernel-level thread count matches.
+void wait_for_thread_cleanup(unsigned int expected) {
+#ifdef __APPLE__
+  while (true) {
+    thread_array_t thread_list;
+    mach_msg_type_number_t count;
+    kern_return_t kr = task_threads(mach_task_self(), &thread_list, &count);
+    if (kr == KERN_SUCCESS) {
+      vm_deallocate(mach_task_self(), (vm_address_t)thread_list,
+                    count * sizeof(thread_t));
+      if (count <= expected)
+        break;
+    }
+    std::this_thread::yield();
+  }
+#endif
+}
diff --git a/lldb/test/API/functionalities/thread/thread_exit/main.cpp b/lldb/test/API/functionalities/thread/thread_exit/main.cpp
index 6c60a40bb66b0..162fec6e296cc 100644
--- a/lldb/test/API/functionalities/thread/thread_exit/main.cpp
+++ b/lldb/test/API/functionalities/thread/thread_exit/main.cpp
@@ -1,5 +1,6 @@
 // This test verifies the correct handling of child thread exits.
 
+#include "mach_thread.h"
 #include "pseudo_barrier.h"
 #include <thread>
 
@@ -7,59 +8,56 @@ pseudo_barrier_t g_barrier1;
 pseudo_barrier_t g_barrier2;
 pseudo_barrier_t g_barrier3;
 
-void *
-thread1 ()
-{
-    // Synchronize with the main thread.
-    pseudo_barrier_wait(g_barrier1);
+void *thread1() {
+  // Synchronize with the main thread.
+  pseudo_barrier_wait(g_barrier1);
 
-    // Synchronize with the main thread and thread2.
-    pseudo_barrier_wait(g_barrier2);
+  // Synchronize with the main thread and thread2.
+  pseudo_barrier_wait(g_barrier2);
 
-    // Return
-    return NULL;                                      // Set second breakpoint here
+  // Return
+  return NULL; // Set second breakpoint here
 }
 
-void *
-thread2 ()
-{
-    // Synchronize with thread1 and the main thread.
-    pseudo_barrier_wait(g_barrier2);
+void *thread2() {
+  // Synchronize with thread1 and the main thread.
+  pseudo_barrier_wait(g_barrier2);
 
-    // Synchronize with the main thread.
-    pseudo_barrier_wait(g_barrier3);
+  // Synchronize with the main thread.
+  pseudo_barrier_wait(g_barrier3);
 
-    // Return
-    return NULL;
+  // Return
+  return NULL;
 }
 
-int main ()
-{
-    pseudo_barrier_init(g_barrier1, 2);
-    pseudo_barrier_init(g_barrier2, 3);
-    pseudo_barrier_init(g_barrier3, 2);
+int main() {
+  pseudo_barrier_init(g_barrier1, 2);
+  pseudo_barrier_init(g_barrier2, 3);
+  pseudo_barrier_init(g_barrier3, 2);
 
-    // Create a thread.
-    std::thread thread_1(thread1);
+  // Create a thread.
+  std::thread thread_1(thread1);
 
-    // Wait for thread1 to start.
-    pseudo_barrier_wait(g_barrier1);
+  // Wait for thread1 to start.
+  pseudo_barrier_wait(g_barrier1);
 
-    // Create another thread.
-    std::thread thread_2(thread2);  // Set first breakpoint here
+  // Create another thread.
+  std::thread thread_2(thread2); // Set first breakpoint here
 
-    // Wait for thread2 to start.
-    pseudo_barrier_wait(g_barrier2);
+  // Wait for thread2 to start.
+  pseudo_barrier_wait(g_barrier2);
 
-    // Wait for the first thread to finish
-    thread_1.join();
+  // Wait for the first thread to finish
+  thread_1.join();
+  wait_for_thread_cleanup(2);
 
-    // Synchronize with the remaining thread
-    int dummy = 47;                   // Set third breakpoint here
-    pseudo_barrier_wait(g_barrier3);
+  // Synchronize with the remaining thread
+  int dummy = 47; // Set third breakpoint here
+  pseudo_barrier_wait(g_barrier3);
 
-    // Wait for the second thread to finish
-    thread_2.join();
+  // Wait for the second thread to finish
+  thread_2.join();
+  wait_for_thread_cleanup(1);
 
-    return 0;                                         // Set fourth breakpoint here
+  return 0; // Set fourth breakpoint here
 }

``````````

</details>


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