From 945bdb167ff5d73559780acc2391dabb816ce9e1 Mon Sep 17 00:00:00 2001 From: Jordan Rupprecht Date: Fri, 2 Sep 2022 10:21:23 -0700 Subject: [PATCH] [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 --- lldb/unittests/Host/MainLoopTest.cpp | 23 +++++++---------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/lldb/unittests/Host/MainLoopTest.cpp b/lldb/unittests/Host/MainLoopTest.cpp index fc79c0e..7c12c52 100644 --- a/lldb/unittests/Host/MainLoopTest.cpp +++ b/lldb/unittests/Host/MainLoopTest.cpp @@ -32,17 +32,13 @@ public: ASSERT_TRUE(error.Success()); Socket *accept_socket; - std::future accept_error = std::async(std::launch::async, [&] { - return listen_socket_up->Accept(accept_socket); - }); - std::unique_ptr 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); -- 2.7.4