aboutsummaryrefslogtreecommitdiff
path: root/src/client
diff options
context:
space:
mode:
authorvapier@chromium.org <vapier@chromium.org@4c0a9323-5329-0410-9bdc-e9ce6186880e>2014-04-02 22:55:12 +0000
committervapier@chromium.org <vapier@chromium.org@4c0a9323-5329-0410-9bdc-e9ce6186880e>2014-04-02 22:55:12 +0000
commit0ac94ba6174d3d5e3725da69d092373d51370181 (patch)
tree62bd5c0d000d4330ad7ca72da46d7eedcc0e5aac /src/client
parentUpdate GYP dependency to r1886. (diff)
downloadbreakpad-0ac94ba6174d3d5e3725da69d092373d51370181.tar.xz
fix races in CrashGenerator::CreateChildCrash
The current CreateChildCrash logic is racy when it comes to creating a crash dump for two reasons: The main thread that calls kill() on a different thread is guaranteed the signal will be *queued* when it returns, but not *delivered*. If the kernel doesn't automatically schedule the receiving thread, but instead lets the main thread run to the exit() call, then the signal never triggers a coredump and the whole process simply exits. The main thread is using kill() to try to deliver a signal to a specific thread, but that function is for sending signals to a process. That means the kernel is free to deliver the signal to any thread in the process and not just the one requested. This manifests itself as the pr_pid in the coredump not being the one expected. Instead, we must use tkill() with the tid (which we already took care of gathering) to deliver to a specific thread. These are a lot easier to see on a UMP system as contention is heavier. BUG=chromium:207918 TEST=`dumper_unittest` still passes, and doesn't flake out in a UMP system TEST=`linux_client_unittest` still passes R=benchan@chromium.org Review URL: https://breakpad.appspot.com/1304005 git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@1299 4c0a9323-5329-0410-9bdc-e9ce6186880e
Diffstat (limited to 'src/client')
-rw-r--r--src/client/linux/minidump_writer/linux_core_dumper_unittest.cc10
1 files changed, 2 insertions, 8 deletions
diff --git a/src/client/linux/minidump_writer/linux_core_dumper_unittest.cc b/src/client/linux/minidump_writer/linux_core_dumper_unittest.cc
index 7aef4c0d..449aab3f 100644
--- a/src/client/linux/minidump_writer/linux_core_dumper_unittest.cc
+++ b/src/client/linux/minidump_writer/linux_core_dumper_unittest.cc
@@ -74,14 +74,8 @@ TEST(LinuxCoreDumperTest, VerifyDumpWithMultipleThreads) {
const unsigned kCrashThread = 1;
const int kCrashSignal = SIGABRT;
pid_t child_pid;
- // TODO(benchan): Revert to use ASSERT_TRUE once the flakiness in
- // CrashGenerator is identified and fixed.
- if (!crash_generator.CreateChildCrash(kNumOfThreads, kCrashThread,
- kCrashSignal, &child_pid)) {
- fprintf(stderr, "LinuxCoreDumperTest.VerifyDumpWithMultipleThreads test "
- "is skipped due to no core dump generated\n");
- return;
- }
+ ASSERT_TRUE(crash_generator.CreateChildCrash(kNumOfThreads, kCrashThread,
+ kCrashSignal, &child_pid));
const string core_file = crash_generator.GetCoreFilePath();
const string procfs_path = crash_generator.GetDirectoryOfProcFilesCopy();