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(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); + pthread_create(&threads[i], &thread_attributes, &thread_function, &pipefd); } - // 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; - } - 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());