[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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D133181.457487.patch
Type: text/x-patch
Size: 1882 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20220902/ee6c55b1/attachment.bin>