[Lldb-commits] [lldb] r367573 - Fix TestThreadSpecificBreakpoint on Windows

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Thu Aug  1 07:49:54 2019
New Revision: 367573

URL: http://llvm.org/viewvc/llvm-project?rev=367573&view=rev
Log:
Fix TestThreadSpecificBreakpoint on Windows

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.

Differential Revision: https://reviews.llvm.org/D65546

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

Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/TestThreadSpecificBreakpoint.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/TestThreadSpecificBreakpoint.py?rev=367573&r1=367572&r2=367573&view=diff
==============================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/TestThreadSpecificBreakpoint.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/TestThreadSpecificBreakpoint.py Thu Aug  1 07:49:54 2019
@@ -27,7 +27,6 @@ class ThreadSpecificBreakTestCase(TestBa
 
     @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 @@ class ThreadSpecificBreakTestCase(TestBa
         (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 @@ class ThreadSpecificBreakTestCase(TestBa
             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 @@ class ThreadSpecificBreakTestCase(TestBa
             "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")

Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/main.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/main.cpp?rev=367573&r1=367572&r2=367573&view=diff
==============================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/main.cpp (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/main.cpp Thu Aug  1 07:49:54 2019
@@ -5,7 +5,14 @@ void
 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