[Lldb-commits] [lldb] 945bdb1 - [test] Remove problematic thread from MainLoopTest to fix flakiness

Jordan Rupprecht via lldb-commits lldb-commits at lists.llvm.org
Fri Sep 2 10:31:54 PDT 2022


Author: Jordan Rupprecht
Date: 2022-09-02T10:30:56-07:00
New Revision: 945bdb167ff5d73559780acc2391dabb816ce9e1

URL: https://github.com/llvm/llvm-project/commit/945bdb167ff5d73559780acc2391dabb816ce9e1
DIFF: https://github.com/llvm/llvm-project/commit/945bdb167ff5d73559780acc2391dabb816ce9e1.diff

LOG: [test] Remove problematic thread from MainLoopTest to fix flakiness

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

Added: 
    

Modified: 
    lldb/unittests/Host/MainLoopTest.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/unittests/Host/MainLoopTest.cpp b/lldb/unittests/Host/MainLoopTest.cpp
index fc79c0e38a39..7c12c5223123 100644
--- a/lldb/unittests/Host/MainLoopTest.cpp
+++ b/lldb/unittests/Host/MainLoopTest.cpp
@@ -32,17 +32,13 @@ class MainLoopTest : public testing::Test {
     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);


        


More information about the lldb-commits mailing list