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

llvm-svn: 367573

2 files changed, 14 insertions, 15 deletions

diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/TestThreadSpecificBreakpoint.py b/lldb/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/TestThreadSpecificBreakpoint.py
index 487beb89523..4fc86d6d7bb 100644
--- a/lldb/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/TestThreadSpecificBreakpoint.py
+++ b/lldb/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/TestThreadSpecificBreakpoint.py
@@ -27,7 +27,6 @@ class ThreadSpecificBreakTestCase(TestBase):
 
     @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(TestBase):
         (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(TestBase):
             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(TestBase):
             "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")
diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/main.cpp b/lldb/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/main.cpp
index 0509b3d37a7..03e93bd4125 100644
--- a/lldb/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/main.cpp
+++ b/lldb/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/main.cpp
@@ -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