[test] Remove problematic thread from MainLoopTest to fix flakiness
authorJordan Rupprecht <rupprecht@google.com>
Fri, 2 Sep 2022 17:21:23 +0000 (10:21 -0700)
committerJordan Rupprecht <rupprecht@google.com>
Fri, 2 Sep 2022 17:30:56 +0000 (10:30 -0700)
This test, specifically `TwoSignalCallbacks`, can be a little bit flaky, failing in around 5/2000 runs.

POSIX says:

> If the value of pid causes sig to be generated for the sending process, and if sig is not blocked for the calling thread and if no other thread has sig unblocked or is waiting in a sigwait() function for sig, either sig or at least one pending unblocked signal shall be delivered to the sending thread before kill() returns.

The problem is that in test setup, we create a new thread with `std::async` and that is occasionally not cleaned up. This leaves that thread available to eat the signal we're polling for.

The need for this to be async does not apply anymore, so we can just make it synchronous.

This makes the test passes in 10000 runs.

Reviewed By: labath

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

lldb/unittests/Host/MainLoopTest.cpp

index fc79c0e..7c12c52 100644 (file)
@@ -32,17 +32,13 @@ public:
     ASSERT_TRUE(error.Success());
 
     Socket *accept_socket;
-    std::future<Status> accept_error = std::async(std::launch::async, [&] {
-      return listen_socket_up->Accept(accept_socket);
-    });
-
     std::unique_ptr<TCPSocket> connect_socket_up(
         new TCPSocket(true, child_processes_inherit));
     error = connect_socket_up->Connect(
         llvm::formatv("localhost:{0}", listen_socket_up->GetLocalPortNumber())
             .str());
     ASSERT_TRUE(error.Success());
-    ASSERT_TRUE(accept_error.get().Success());
+    ASSERT_TRUE(listen_socket_up->Accept(accept_socket).Success());
 
     callback_count = 0;
     socketpair[0] = std::move(connect_socket_up);
@@ -174,7 +170,7 @@ TEST_F(MainLoopTest, Signal) {
 
   auto handle = loop.RegisterSignal(SIGUSR1, make_callback(), error);
   ASSERT_TRUE(error.Success());
-  kill(getpid(), SIGUSR1);
+  pthread_kill(pthread_self(), SIGUSR1);
   ASSERT_TRUE(loop.Run().Success());
   ASSERT_EQ(1u, callback_count);
 }
@@ -192,14 +188,9 @@ TEST_F(MainLoopTest, UnmonitoredSignal) {
 
   auto handle = loop.RegisterSignal(SIGUSR1, make_callback(), error);
   ASSERT_TRUE(error.Success());
-  std::thread killer([]() {
-    sleep(1);
-    kill(getpid(), SIGUSR2);
-    sleep(1);
-    kill(getpid(), SIGUSR1);
-  });
+  pthread_kill(pthread_self(), SIGUSR2);
+  pthread_kill(pthread_self(), SIGUSR1);
   ASSERT_TRUE(loop.Run().Success());
-  killer.join();
   ASSERT_EQ(1u, callback_count);
 }
 
@@ -220,7 +211,7 @@ TEST_F(MainLoopTest, TwoSignalCallbacks) {
         SIGUSR1, [&](MainLoopBase &loop) { ++callback2_count; }, error);
     ASSERT_TRUE(error.Success());
 
-    kill(getpid(), SIGUSR1);
+    pthread_kill(pthread_self(), SIGUSR1);
     ASSERT_TRUE(loop.Run().Success());
     ASSERT_EQ(1u, callback_count);
     ASSERT_EQ(1u, callback2_count);
@@ -233,7 +224,7 @@ TEST_F(MainLoopTest, TwoSignalCallbacks) {
         SIGUSR1, [&](MainLoopBase &loop) { ++callback3_count; }, error);
     ASSERT_TRUE(error.Success());
 
-    kill(getpid(), SIGUSR1);
+    pthread_kill(pthread_self(), SIGUSR1);
     ASSERT_TRUE(loop.Run().Success());
     ASSERT_EQ(2u, callback_count);
     ASSERT_EQ(1u, callback2_count);
@@ -241,7 +232,7 @@ TEST_F(MainLoopTest, TwoSignalCallbacks) {
   }
 
   // Both extra callbacks should be unregistered now.
-  kill(getpid(), SIGUSR1);
+  pthread_kill(pthread_self(), SIGUSR1);
   ASSERT_TRUE(loop.Run().Success());
   ASSERT_EQ(3u, callback_count);
   ASSERT_EQ(1u, callback2_count);