Fix race in VerifyStackReadWithMultipleThreads

Patch by Chris Dearman <chris@mips.com>
R=ted at http://breakpad.appspot.com/377002

git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@959 4c0a9323-5329-0410-9bdc-e9ce6186880e

2 files changed, 29 insertions, 20 deletions

diff --git a/src/client/linux/minidump_writer/linux_dumper_unittest_helper.cc b/src/client/linux/minidump_writer/linux_dumper_unittest_helper.cc
index 6e3d5e4b..418e7e67 100644
--- a/src/client/linux/minidump_writer/linux_dumper_unittest_helper.cc
+++ b/src/client/linux/minidump_writer/linux_dumper_unittest_helper.cc
@@ -51,7 +51,14 @@
 #endif
 
 void *thread_function(void *data) {
+  int pipefd = *static_cast<int *>(data);
   volatile pid_t thread_id = syscall(__NR_gettid);
+  // Signal parent that a thread has started.
+  uint8_t byte = 1;
+  if (write(pipefd, &byte, sizeof(byte)) != sizeof(byte)) {
+    perror("ERROR: parent notification failed");
+    return NULL;
+  }
   register volatile pid_t *thread_id_ptr asm(TID_PTR_REGISTER) = &thread_id;
   while (true)
     asm volatile ("" : : "r" (thread_id_ptr));
@@ -75,14 +82,8 @@ int main(int argc, char *argv[]) {
   pthread_attr_init(&thread_attributes);
   pthread_attr_setdetachstate(&thread_attributes, PTHREAD_CREATE_DETACHED);
   for (int i = 1; i < num_threads; i++) {
-    pthread_create(&threads[i], &thread_attributes, &thread_function, NULL);
-  }
-  // Signal parent that this process has started all threads.
-  uint8_t byte = 1;
-  if (write(pipefd, &byte, sizeof(byte)) != sizeof(byte)) {
-    perror("ERROR: parent notification failed");
-    return 1;
+    pthread_create(&threads[i], &thread_attributes, &thread_function, &pipefd);
   }
-  thread_function(NULL);
+  thread_function(&pipefd);
   return 0;
 }
diff --git a/src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc b/src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc
index a1665082..ef47da9f 100644
--- a/src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc
+++ b/src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc
@@ -212,20 +212,28 @@ TEST(LinuxPtraceDumperTest, VerifyStackReadWithMultipleThreads) {
     exit(0);
   }
   close(fds[1]);
-  // Wait for the child process to signal that it's ready.
-  struct pollfd pfd;
-  memset(&pfd, 0, sizeof(pfd));
-  pfd.fd = fds[0];
-  pfd.events = POLLIN | POLLERR;
-
-  const int r = HANDLE_EINTR(poll(&pfd, 1, 1000));
-  ASSERT_EQ(1, r);
-  ASSERT_TRUE(pfd.revents & POLLIN);
-  uint8_t junk;
-  read(fds[0], &junk, sizeof(junk));
+
+  // Wait for all child threads to indicate that they have started
+  for (int threads = 0; threads < kNumberOfThreadsInHelperProgram; threads++) {
+    struct pollfd pfd;
+    memset(&pfd, 0, sizeof(pfd));
+    pfd.fd = fds[0];
+    pfd.events = POLLIN | POLLERR;
+
+    const int r = HANDLE_EINTR(poll(&pfd, 1, 1000));
+    ASSERT_EQ(1, r);
+    ASSERT_TRUE(pfd.revents & POLLIN);
+    uint8_t junk;
+    ASSERT_EQ(read(fds[0], &junk, sizeof(junk)), sizeof(junk));
+  }
   close(fds[0]);
 
-  // Child is ready now.
+  // There is a race here because we may stop a child thread before
+  // it is actually running the busy loop. Empirically this sleep
+  // is sufficient to avoid the race.
+  usleep(100000);
+
+  // Children are ready now.
   LinuxPtraceDumper dumper(child_pid);
   ASSERT_TRUE(dumper.Init());
   EXPECT_EQ((size_t)kNumberOfThreadsInHelperProgram, dumper.threads().size());