From 0ac94ba6174d3d5e3725da69d092373d51370181 Mon Sep 17 00:00:00 2001 From: "vapier@chromium.org" Date: Wed, 2 Apr 2014 22:55:12 +0000 Subject: 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 --- src/client/linux/minidump_writer/linux_core_dumper_unittest.cc | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) (limited to 'src/client') 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(); -- cgit v1.2.1