diff options
author | vapier@chromium.org <vapier@chromium.org@4c0a9323-5329-0410-9bdc-e9ce6186880e> | 2014-04-02 22:55:12 +0000 |
---|---|---|
committer | vapier@chromium.org <vapier@chromium.org@4c0a9323-5329-0410-9bdc-e9ce6186880e> | 2014-04-02 22:55:12 +0000 |
commit | 0ac94ba6174d3d5e3725da69d092373d51370181 (patch) | |
tree | 62bd5c0d000d4330ad7ca72da46d7eedcc0e5aac /src/common/linux/elf_core_dump_unittest.cc | |
parent | Update GYP dependency to r1886. (diff) | |
download | breakpad-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/common/linux/elf_core_dump_unittest.cc')
-rw-r--r-- | src/common/linux/elf_core_dump_unittest.cc | 10 |
1 files changed, 2 insertions, 8 deletions
diff --git a/src/common/linux/elf_core_dump_unittest.cc b/src/common/linux/elf_core_dump_unittest.cc index 45aa5364..7c122aa6 100644 --- a/src/common/linux/elf_core_dump_unittest.cc +++ b/src/common/linux/elf_core_dump_unittest.cc @@ -138,14 +138,8 @@ TEST(ElfCoreDumpTest, ValidCoreFile) { const unsigned kNumOfThreads = 3; const unsigned kCrashThread = 1; const int kCrashSignal = SIGABRT; - // TODO(benchan): Revert to use ASSERT_TRUE once the flakiness in - // CrashGenerator is identified and fixed. - if (!crash_generator.CreateChildCrash(kNumOfThreads, kCrashThread, - kCrashSignal, NULL)) { - fprintf(stderr, "ElfCoreDumpTest.ValidCoreFile test is skipped " - "due to no core dump generated"); - return; - } + ASSERT_TRUE(crash_generator.CreateChildCrash(kNumOfThreads, kCrashThread, + kCrashSignal, NULL)); pid_t expected_crash_thread_id = crash_generator.GetThreadId(kCrashThread); set<pid_t> expected_thread_ids; for (unsigned i = 0; i < kNumOfThreads; ++i) { |