[Lldb-commits] [PATCH] D133181: [test] Ensure MainLoop has time to start listening for signals.

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht created this revision.
Herald added a project: All.
rupprecht requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This test, specifically `TwoSignalCallbacks`, can be a little bit flaky, 
failing in around 5/2000 runs. My suspicion is around this pattern:

  kill(getpid(), SIGUSR1);
  ASSERT_TRUE(loop.Run().Success());

First we send the signal, and _then_ we call `loop.Run()`, which resets the 
"please terminate me" bit and waits for the signal handler to flip it on again. 
AFAICT `kill` is entirely asynchronous and not much is happening here, and so 
we almost always get to the point in `loop.Run()` where we spin and wait for 
termination. But sometimes we get unlucky, the signal gets delivered before we 
start waiting for it, and this test deadlocks.

This changes the test to sleep a second before sending the signal, and the test 
passes in 10000 runs.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133181

Files:
  lldb/unittests/Host/MainLoopTest.cpp


Index: lldb/unittests/Host/MainLoopTest.cpp
===================================================================
--- lldb/unittests/Host/MainLoopTest.cpp
+++ lldb/unittests/Host/MainLoopTest.cpp
@@ -174,8 +174,13 @@
 
   auto handle = loop.RegisterSignal(SIGUSR1, make_callback(), error);
   ASSERT_TRUE(error.Success());
-  kill(getpid(), SIGUSR1);
+  std::thread killer([]() {
+    sleep(1);
+    kill(getpid(), SIGUSR1);
+  });
   ASSERT_TRUE(loop.Run().Success());
+  killer.join();
+
   ASSERT_EQ(1u, callback_count);
 }
 
@@ -220,8 +225,12 @@
         SIGUSR1, [&](MainLoopBase &loop) { ++callback2_count; }, error);
     ASSERT_TRUE(error.Success());
 
-    kill(getpid(), SIGUSR1);
+    std::thread killer([]() {
+      sleep(1);
+      kill(getpid(), SIGUSR1);
+    });
     ASSERT_TRUE(loop.Run().Success());
+    killer.join();
     ASSERT_EQ(1u, callback_count);
     ASSERT_EQ(1u, callback2_count);
     ASSERT_EQ(0u, callback3_count);
@@ -233,18 +242,28 @@
         SIGUSR1, [&](MainLoopBase &loop) { ++callback3_count; }, error);
     ASSERT_TRUE(error.Success());
 
-    kill(getpid(), SIGUSR1);
+    std::thread killer([]() {
+      sleep(1);
+      kill(getpid(), SIGUSR1);
+    });
     ASSERT_TRUE(loop.Run().Success());
+    killer.join();
     ASSERT_EQ(2u, callback_count);
     ASSERT_EQ(1u, callback2_count);
     ASSERT_EQ(1u, callback3_count);
   }
 
   // Both extra callbacks should be unregistered now.
-  kill(getpid(), SIGUSR1);
-  ASSERT_TRUE(loop.Run().Success());
-  ASSERT_EQ(3u, callback_count);
-  ASSERT_EQ(1u, callback2_count);
-  ASSERT_EQ(1u, callback3_count);
+  {
+    std::thread killer([]() {
+      sleep(1);
+      kill(getpid(), SIGUSR1);
+    });
+    ASSERT_TRUE(loop.Run().Success());
+    killer.join();
+    ASSERT_EQ(3u, callback_count);
+    ASSERT_EQ(1u, callback2_count);
+    ASSERT_EQ(1u, callback3_count);
+  }
 }
 #endif



Index: lldb/unittests/Host/MainLoopTest.cpp
===================================================================
--- lldb/unittests/Host/MainLoopTest.cpp
+++ lldb/unittests/Host/MainLoopTest.cpp
@@ -174,8 +174,13 @@
 
   auto handle = loop.RegisterSignal(SIGUSR1, make_callback(), error);
   ASSERT_TRUE(error.Success());
-  kill(getpid(), SIGUSR1);
+  std::thread killer([]() {
+    sleep(1);
+    kill(getpid(), SIGUSR1);
+  });
   ASSERT_TRUE(loop.Run().Success());
+  killer.join();
+
   ASSERT_EQ(1u, callback_count);
 }
 
@@ -220,8 +225,12 @@
         SIGUSR1, [&](MainLoopBase &loop) { ++callback2_count; }, error);
     ASSERT_TRUE(error.Success());
 
-    kill(getpid(), SIGUSR1);
+    std::thread killer([]() {
+      sleep(1);
+      kill(getpid(), SIGUSR1);
+    });
     ASSERT_TRUE(loop.Run().Success());
+    killer.join();
     ASSERT_EQ(1u, callback_count);
     ASSERT_EQ(1u, callback2_count);
     ASSERT_EQ(0u, callback3_count);
@@ -233,18 +242,28 @@
         SIGUSR1, [&](MainLoopBase &loop) { ++callback3_count; }, error);
     ASSERT_TRUE(error.Success());
 
-    kill(getpid(), SIGUSR1);
+    std::thread killer([]() {
+      sleep(1);
+      kill(getpid(), SIGUSR1);
+    });
     ASSERT_TRUE(loop.Run().Success());
+    killer.join();
     ASSERT_EQ(2u, callback_count);
     ASSERT_EQ(1u, callback2_count);
     ASSERT_EQ(1u, callback3_count);
   }
 
   // Both extra callbacks should be unregistered now.
-  kill(getpid(), SIGUSR1);
-  ASSERT_TRUE(loop.Run().Success());
-  ASSERT_EQ(3u, callback_count);
-  ASSERT_EQ(1u, callback2_count);
-  ASSERT_EQ(1u, callback3_count);
+  {
+    std::thread killer([]() {
+      sleep(1);
+      kill(getpid(), SIGUSR1);
+    });
+    ASSERT_TRUE(loop.Run().Success());
+    killer.join();
+    ASSERT_EQ(3u, callback_count);
+    ASSERT_EQ(1u, callback2_count);
+    ASSERT_EQ(1u, callback3_count);
+  }
 }
 #endif

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits