From 52205b820693b74fb2b33beef2f4cd4ba63cf738 Mon Sep 17 00:00:00 2001
From: Frederick Mayle <fmayle@google.com>
Date: Thu, 15 Dec 2022 16:08:52 -0800
Subject: [PATCH] debuggerd_client_test: less racy test setup

Make sure that all the threads have started up, otherwise the main part
of the test might not be testing as stressful a situation as expected.
Note that the "race" moniker is still valid because of the debuggerd
timeout.

The test is now faster (405ms) when run under good conditions.

Test: atest 'debuggerd_test:debuggerd_client#race'
Test: Ran debuggerd_client.race 1000 times on its own.
Test: Ran the whole suite of debuggerd unit tests 1000 times.
Change-Id: I487e7654a71df9f1799f09c6f385c929ddf2f234
---
 debuggerd/client/debuggerd_client_test.cpp | 30 ++++++++++++++--------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/debuggerd/client/debuggerd_client_test.cpp b/debuggerd/client/debuggerd_client_test.cpp
index ebb8d86e0..33ff05fd6 100644
--- a/debuggerd/client/debuggerd_client_test.cpp
+++ b/debuggerd/client/debuggerd_client_test.cpp
@@ -18,6 +18,7 @@
 
 #include <fcntl.h>
 #include <stdio.h>
+#include <sys/eventfd.h>
 #include <unistd.h>
 
 #include <chrono>
@@ -51,23 +52,35 @@ static int getThreadCount() {
 
 TEST(debuggerd_client, race) {
   static int THREAD_COUNT = getThreadCount();
+
+  // Semaphore incremented once per thread started.
+  unique_fd barrier(eventfd(0, EFD_SEMAPHORE));
+  ASSERT_NE(-1, barrier.get());
+
   pid_t forkpid = fork();
-
   ASSERT_NE(-1, forkpid);
-
   if (forkpid == 0) {
     // Spawn a bunch of threads, to make crash_dump take longer.
     std::vector<std::thread> threads;
+    threads.reserve(THREAD_COUNT);
     for (int i = 0; i < THREAD_COUNT; ++i) {
-      threads.emplace_back([]() {
-        while (true) {
-          std::this_thread::sleep_for(60s);
+      threads.emplace_back([&barrier]() {
+        uint64_t count = 1;
+        ASSERT_NE(-1, write(barrier.get(), &count, sizeof(count)));
+        for (;;) {
+          pause();
         }
       });
     }
+    for (;;) {
+      pause();
+    }
+  }
 
-    std::this_thread::sleep_for(60s);
-    exit(0);
+  // Wait for the child to spawn all of its threads.
+  for (int i = 0; i < THREAD_COUNT; ++i) {
+    uint64_t count;
+    ASSERT_NE(-1, read(barrier.get(), &count, sizeof(count)));
   }
 
   unique_fd pipe_read, pipe_write;
@@ -77,9 +90,6 @@ TEST(debuggerd_client, race) {
   constexpr int PIPE_SIZE = 16 * 1024 * 1024;
   ASSERT_EQ(PIPE_SIZE, fcntl(pipe_read.get(), F_SETPIPE_SZ, PIPE_SIZE));
 
-  // Wait for a bit to let the child spawn all of its threads.
-  std::this_thread::sleep_for(1s);
-
   ASSERT_TRUE(
       debuggerd_trigger_dump(forkpid, kDebuggerdNativeBacktrace, 60000, std::move(pipe_write)));
   // Immediately kill the forked child, to make sure that the dump didn't return early.