Logging data race in SecurityExploitBrowserTest.* and SiteIsolationStatsGathererBrowserTest.* |
||||
Issue descriptionLooks like the issue is that content::SiteIsolationStatsGathererBrowserTest::SetUpCommandLine() creates a thread. This happens before content::ContentBrowserTest::SetUp() calls content::BrowserTestBase::SetUp() which ends up initializing logging. Therefore logging operation on the created thread are racy. Creating a thread from SetUpCommandLine() sounds wrong (should be in SetUp() after calling the parent's SetUp()?). And if we can't help it, then we should call content::BrowserTestBase::SetUp() as the first thing in content::ContentBrowserTest::SetUp() (setting up the parent before self is also a common paradigm). Closest blame I can find is a year old... https://codereview.chromium.org/1411073005 (@svaldez) Use CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_tsan_rel_ng to see these errors in try bots. Example TSAN log: SecurityExploitBrowserTest.AttemptDuplicateRenderViewHost (run #1): [ RUN ] SecurityExploitBrowserTest.AttemptDuplicateRenderViewHost ================== WARNING: ThreadSanitizer: data race (pid=28554) Read of size 4 at 0x00000a778da4 by thread T1: #0 logging::ShouldCreateLogMessage(int) base/logging.cc:417:10 (content_browsertests+0x00000305cc87) #1 operator-> base/memory/ref_counted.h:322:5 (content_browsertests+0x00000306f2d8) #2 ReloadWorkQueue base/message_loop/message_loop.cc:478 (content_browsertests+0x00000306f2d8) #3 base::MessageLoop::DoWork() base/message_loop/message_loop.cc:499 (content_browsertests+0x00000306f2d8) #4 base::MessagePumpLibevent::Run(base::MessagePump::Delegate*) base/message_loop/message_pump_libevent.cc:218:31 (content_browsertests+0x000003075e60) #5 base::MessageLoop::RunHandler() base/message_loop/message_loop.cc:378:10 (content_browsertests+0x00000306df72) #6 base::RunLoop::Run() base/run_loop.cc:37:10 (content_browsertests+0x0000030a9b0e) #7 base::Thread::Run(base::RunLoop*) base/threading/thread.cc:245:13 (content_browsertests+0x0000030e86ab) #8 base::Thread::ThreadMain() base/threading/thread.cc:328:3 (content_browsertests+0x0000030e8db6) #9 base::(anonymous namespace)::ThreadFunc(void*) base/threading/platform_thread_posix.cc:71:13 (content_browsertests+0x0000030de6b8) Previous write of size 4 at 0x00000a778da4 by main thread: #0 logging::BaseInitLoggingImpl_built_with_NDEBUG(logging::LoggingSettings const&) base/logging.cc:378:25 (content_browsertests+0x00000305c934) #1 InitLogging base/logging.h:232:10 (content_browsertests+0x000002ae705d) #2 InitLogging content/shell/app/shell_main_delegate.cc:111 (content_browsertests+0x000002ae705d) #3 content::ShellMainDelegate::BasicStartupComplete(int*) content/shell/app/shell_main_delegate.cc:145 (content_browsertests+0x000002ae705d) #4 content::ContentMainRunnerImpl::Initialize(content::ContentMainParams const&) content/app/content_main_runner.cc:558:33 (content_browsertests+0x0000020c6c49) #5 content::ContentMain(content::ContentMainParams const&) content/app/content_main.cc:16:32 (content_browsertests+0x0000020bca1e) #6 content::BrowserTestBase::SetUp() content/public/test/browser_test_base.cc:316:3 (content_browsertests+0x000002a7d6ab) #7 content::ContentBrowserTest::SetUp() content/public/test/content_browser_test.cc:94:20 (content_browsertests+0x000002a76b66) #8 HandleExceptionsInMethodIfSupported<testing::Test, void> testing/gtest/src/gtest.cc:2458:12 (content_browsertests+0x000002d31f30) #9 testing::Test::Run() testing/gtest/src/gtest.cc:2470 (content_browsertests+0x000002d31f30) #10 testing::TestInfo::Run() testing/gtest/src/gtest.cc:2656:11 (content_browsertests+0x000002d332fc) #11 testing::TestCase::Run() testing/gtest/src/gtest.cc:2774:28 (content_browsertests+0x000002d33bd6) #12 testing::internal::UnitTestImpl::RunAllTests() testing/gtest/src/gtest.cc:4647:43 (content_browsertests+0x000002d3eae6) #13 HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> testing/gtest/src/gtest.cc:2458:12 (content_browsertests+0x000002d3e379) #14 testing::UnitTest::Run() testing/gtest/src/gtest.cc:4255 (content_browsertests+0x000002d3e379) #15 RUN_ALL_TESTS testing/gtest/include/gtest/gtest.h:2237:46 (content_browsertests+0x000002ab47c9) #16 base::TestSuite::Run() base/test/test_suite.cc:271 (content_browsertests+0x000002ab47c9) #17 content::ContentTestLauncherDelegate::RunTestSuite(int, char**) content/test/content_test_launcher.cc:105:48 (content_browsertests+0x000002a7c70b) #18 content::LaunchTests(content::TestLauncherDelegate*, int, int, char**) content/public/test/test_launcher.cc:520:31 (content_browsertests+0x000002a9e663) #19 main content/test/content_test_launcher.cc:131:10 (content_browsertests+0x000002a7c692) Location is global 'logging::(anonymous namespace)::g_logging_destination' of size 4 at 0x00000a778da4 (content_browsertests+0x00000a778da4) Thread T1 'EmbeddedTestServer IO Thread' (tid=28566, running) created by main thread at: #0 pthread_create <null> (content_browsertests+0x0000004b4d75) #1 base::(anonymous namespace)::CreateThread(unsigned long, bool, base::PlatformThread::Delegate*, base::PlatformThreadHandle*, base::ThreadPriority) base/threading/platform_thread_posix.cc:110:13 (content_browsertests+0x0000030de166) #2 base::PlatformThread::CreateWithPriority(unsigned long, base::PlatformThread::Delegate*, base::PlatformThreadHandle*, base::ThreadPriority) base/threading/platform_thread_posix.cc:191:10 (content_browsertests+0x0000030de025) #3 base::Thread::StartWithOptions(base::Thread::Options const&) base/threading/thread.cc:112:15 (content_browsertests+0x0000030e7d83) #4 net::test_server::EmbeddedTestServer::StartAcceptingConnections() net/test/embedded_test_server/embedded_test_server.cc:172:3 (content_browsertests+0x000002c11e64) #5 net::test_server::EmbeddedTestServer::Start() net/test/embedded_test_server/embedded_test_server.cc:89:3 (content_browsertests+0x000002c11645) #6 content::SecurityExploitBrowserTest::SetUpCommandLine(base::CommandLine*) content/browser/security_exploit_browsertest.cc:183:5 (content_browsertests+0x00000087a8b4) #7 content::ContentBrowserTest::SetUp() content/public/test/content_browser_test.cc:61:3 (content_browsertests+0x000002a76b59) #8 HandleExceptionsInMethodIfSupported<testing::Test, void> testing/gtest/src/gtest.cc:2458:12 (content_browsertests+0x000002d31f30) #9 testing::Test::Run() testing/gtest/src/gtest.cc:2470 (content_browsertests+0x000002d31f30) #10 testing::TestInfo::Run() testing/gtest/src/gtest.cc:2656:11 (content_browsertests+0x000002d332fc) #11 testing::TestCase::Run() testing/gtest/src/gtest.cc:2774:28 (content_browsertests+0x000002d33bd6) #12 testing::internal::UnitTestImpl::RunAllTests() testing/gtest/src/gtest.cc:4647:43 (content_browsertests+0x000002d3eae6) #13 HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> testing/gtest/src/gtest.cc:2458:12 (content_browsertests+0x000002d3e379) #14 testing::UnitTest::Run() testing/gtest/src/gtest.cc:4255 (content_browsertests+0x000002d3e379) #15 RUN_ALL_TESTS testing/gtest/include/gtest/gtest.h:2237:46 (content_browsertests+0x000002ab47c9) #16 base::TestSuite::Run() base/test/test_suite.cc:271 (content_browsertests+0x000002ab47c9) #17 content::ContentTestLauncherDelegate::RunTestSuite(int, char**) content/test/content_test_launcher.cc:105:48 (content_browsertests+0x000002a7c70b) #18 content::LaunchTests(content::TestLauncherDelegate*, int, int, char**) content/public/test/test_launcher.cc:520:31 (content_browsertests+0x000002a9e663) #19 main content/test/content_test_launcher.cc:131:10 (content_browsertests+0x000002a7c692) SUMMARY: ThreadSanitizer: data race base/logging.cc:417:10 in logging::ShouldCreateLogMessage(int)
,
Jan 13 2017
https://codereview.chromium.org/2630843002
,
Jan 13 2017
,
Jan 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0fa95f0d5127fbf230f3d6cea29376f2a0c12c74 commit 0fa95f0d5127fbf230f3d6cea29376f2a0c12c74 Author: gab <gab@chromium.org> Date: Sat Jan 14 01:38:06 2017 Fix race in SecurityExploitBrowserTest and SiteIsolationStatsGathererBrowserTest BUG= 674545 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_tsan_rel_ng Review-Url: https://codereview.chromium.org/2630843002 Cr-Commit-Position: refs/heads/master@{#443761} [modify] https://crrev.com/0fa95f0d5127fbf230f3d6cea29376f2a0c12c74/content/browser/security_exploit_browsertest.cc [modify] https://crrev.com/0fa95f0d5127fbf230f3d6cea29376f2a0c12c74/content/child/site_isolation_stats_gatherer_browsertest.cc
,
Jan 14 2017
|
||||
►
Sign in to add a comment |
||||
Comment 1 by gab@chromium.org
, Jan 13 2017