Skip to content

Commit

Permalink
[test] Remove problematic thread from MainLoopTest to fix flakiness
Browse files Browse the repository at this point in the history
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
  • Loading branch information
rupprecht committed Sep 2, 2022
1 parent a931edd commit 945bdb1
Showing 1 changed file with 7 additions and 16 deletions.
23 changes: 7 additions & 16 deletions lldb/unittests/Host/MainLoopTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand 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);
}
Expand All @@ -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);
}

Expand All @@ -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);
Expand All @@ -233,15 +224,15 @@ 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);
ASSERT_EQ(1u, callback3_count);
}

// 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);
Expand Down

0 comments on commit 945bdb1

Please sign in to comment.