aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorted.mielczarek <ted.mielczarek@4c0a9323-5329-0410-9bdc-e9ce6186880e>2012-04-25 11:42:52 +0000
committerted.mielczarek <ted.mielczarek@4c0a9323-5329-0410-9bdc-e9ce6186880e>2012-04-25 11:42:52 +0000
commite6e778f635a1611f4d65145ce83255094d5db272 (patch)
tree1a89160e1d8158719db0a8aa57d3f919d54dce00
parentFix compiler warning from format string (diff)
downloadbreakpad-e6e778f635a1611f4d65145ce83255094d5db272.tar.xz
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
-rw-r--r--src/client/linux/minidump_writer/linux_dumper_unittest_helper.cc17
-rw-r--r--src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc32
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());