[Lldb-commits] [PATCH] D65546: Fix TestThreadSpecificBreakpoint on Windows

[Adrian McCarthy via Phabricator via lldb-commits]

amccarth created this revision.
amccarth added a reviewer: stella.stamenova.

This test was frequently hanging on Windows, causing a timeout after 10 minutes.  The short delay (100 microsecond) in the sample program could cause a deadlock in the Windows thread pool, as I've explained in the test program's comments.

Now that it doesn't hang, it passes reliably, so I've removed the Windows-specific XFAIL.

I've tried to clarify the comments in TestThreadSpecificGBreakpoint.py by eliminating some redundancy and typos, and I simplified away a couple unnecessary assignments.


https://reviews.llvm.org/D65546

Files:
  lldb/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/TestThreadSpecificBreakpoint.py
  lldb/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/main.cpp


Index: lldb/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/main.cpp
===================================================================
--- lldb/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/main.cpp
+++ lldb/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/main.cpp
@@ -5,7 +5,14 @@
 thread_function ()
 {
     // Set thread-specific breakpoint here.
-    std::this_thread::sleep_for(std::chrono::microseconds(100));
+    std::this_thread::sleep_for(std::chrono::milliseconds(20));
+    // On Windows, a sleep_for of less than about 16 ms effectively calls
+    // Sleep(0).  The MS standard thread implementation uses a system thread
+    // pool, which can deadlock on a Sleep(0), hanging not only the secondary
+    // thread but the entire test.  I increased the delay to 20 ms to ensure
+    // Sleep is called with a delay greater than 0.  The deadlock potential
+    // is described here:
+    // https://docs.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-sleep#remarks
 }
 
 int 
Index: lldb/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/TestThreadSpecificBreakpoint.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/TestThreadSpecificBreakpoint.py
+++ lldb/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/TestThreadSpecificBreakpoint.py
@@ -27,7 +27,6 @@
 
     @add_test_categories(['pyapi'])
 
-    @expectedFailureAll(oslist=["windows"])
     @expectedFailureAll(oslist=['ios', 'watchos', 'tvos', 'bridgeos'], archs=['armv7', 'armv7k'], bugnumber='rdar://problem/34563920') # armv7 ios problem - breakpoint with tid qualifier isn't working
     @expectedFailureNetBSD
     def test_thread_id(self):
@@ -46,13 +45,6 @@
         (target, process, main_thread, main_breakpoint) = lldbutil.run_to_source_breakpoint(self,
                 "Set main breakpoint here", main_source_spec)
 
-        main_thread_id = main_thread.GetThreadID()
-
-        # This test works by setting a breakpoint in a function conditioned to stop only on
-        # the main thread, and then calling this function on a secondary thread, joining,
-        # and then calling again on the main thread.  If the thread specific breakpoint works
-        # then it should not be hit on the secondary thread, only on the main
-        # thread.
         thread_breakpoint = target.BreakpointCreateBySourceRegex(
             "Set thread-specific breakpoint here", main_source_spec)
         self.assertGreater(
@@ -60,11 +52,11 @@
             0,
             "thread breakpoint has no locations associated with it.")
 
-        # Set the thread-specific breakpoint to only stop on the main thread.  The run the function
-        # on another thread and join on it.  If the thread-specific breakpoint works, the next
-        # stop should be on the main thread.
-
-        main_thread_id = main_thread.GetThreadID()
+        # Set the thread-specific breakpoint to stop only on the main thread
+        # before the secondary thread has a chance to execute it.  The main
+        # thread joins the secondary thread, and then the main thread will
+        # execute the code at the breakpoint.  If the thread-specific
+        # breakpoint works, the next stop will be on the main thread.
         setter_method(main_thread, thread_breakpoint)
 
         process.Continue()
@@ -81,5 +73,5 @@
             "thread breakpoint stopped at unexpected number of threads")
         self.assertEqual(
             stopped_threads[0].GetThreadID(),
-            main_thread_id,
+            main_thread.GetThreadID(),
             "thread breakpoint stopped at the wrong thread")


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D65546.212681.patch
Type: text/x-patch
Size: 3816 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20190731/80ca6b1a/attachment.bin>