Data race in DownloadContentTest.ForceDownloadMessageRfc822Page |
|||||
Issue descriptionhttps://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=content_browsertests&tests=DownloadContentTest.ForceDownloadMessageRfc822Page shows a couple of failures for this test on the TSAN bot: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20TSan%20Tests/20712 https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20TSan%20Tests/20678 (succeeded on a retry) [ RUN ] DownloadContentTest.ForceDownloadMessageRfc822Page Xlib: extension "RANDR" missing on display ":99". DevTools listening on ws://127.0.0.1:39325/devtools/browser/5f0cc992-cbea-490e-90af-498e1cecd041 ================== WARNING: ThreadSanitizer: data race (pid=11191) Write of size 8 at 0x7b100001c8c8 by main thread: #0 SetBrowser content/public/common/content_client.cc:22:24 (content_browsertests+0x2949505) #1 content::SetBrowserClientForTesting(content::ContentBrowserClient*) content/public/common/content_client.cc:48 (content_browsertests+0x2949505) #2 content::DownloadContentTest_ForceDownloadMessageRfc822Page_Test::RunTestOnMainThread() content/browser/download/download_browsertest.cc:3403:3 (content_browsertests+0x184a18b) ... Previous read of size 8 at 0x7b100001c8c8 by thread T15: #0 browser content/public/common/content_client.h:78:44 (content_browsertests+0x4343e4e) #1 content::internal::CreateDefaultPosixFilesToMap(int, mojo::edk::PlatformHandle const&, bool, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, base::CommandLine*) content/browser/child_process_launcher_helper_posix.cc:96 (content_browsertests+0x4343e4e) #2 content::internal::ChildProcessLauncherHelper::GetFilesToMap() content/browser/child_process_launcher_helper_linux.cc:41:10 (content_browsertests+0x3d84a9d) #3 content::internal::ChildProcessLauncherHelper::LaunchOnLauncherThread() content/browser/child_process_launcher_helper.cc:110:60 (content_browsertests+0x3d83e7a) ... Location is heap block of size 64 at 0x7b100001c8c0 allocated by main thread: #0 operator new(unsigned long) /b/build/slave/linux_upload_clang/build/src/third_party/llvm/compiler-rt/lib/tsan/rtl/tsan_new_delete.cc:57:3 (content_browsertests+0x1711349) #1 content::ShellMainDelegate::BasicStartupComplete(int*) content/shell/app/shell_main_delegate.cc (content_browsertests+0x4737d4a) #2 content::ContentMainRunnerImpl::Initialize(content::ContentMainParams const&) content/app/content_main_runner.cc:729:33 (content_browsertests+0x3bdf51e) #3 content::ContentServiceManagerMainDelegate::Initialize(service_manager::MainDelegate::InitializeParams const&) content/app/content_service_manager_main_delegate.cc:37:32 (content_browsertests+0x2d23207) #4 service_manager::Main(service_manager::MainParams const&) services/service_manager/embedder/main.cc:409:29 (content_browsertests+0x67c8d25) #5 content::ContentMain(content::ContentMainParams const&) content/app/content_main.cc:19:10 (content_browsertests+0x2d23c0e) ... Thread T15 'TaskSchedulerSi' (tid=11259, running) created by thread T2 at: #0 pthread_create /b/build/slave/linux_upload_clang/build/src/third_party/llvm/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc:965:3 (content_browsertests+0x16a7785) #1 base::(anonymous namespace)::CreateThread(unsigned long, bool, base::PlatformThread::Delegate*, base::PlatformThreadHandle*, base::ThreadPriority) base/threading/platform_thread_posix.cc:115:13 (content_browsertests+0x4c472c7) #2 base::PlatformThread::CreateWithPriority(unsigned long, base::PlatformThread::Delegate*, base::PlatformThreadHandle*, base::ThreadPriority) base/threading/platform_thread_posix.cc:200:10 (content_browsertests+0x4c471c5) #3 base::internal::SchedulerWorker::Start() base/task_scheduler/scheduler_worker.cc:67:3 (content_browsertests+0x4bd307f) #4 CreateTaskRunnerWithTraitsImpl<base::internal::(anonymous namespace)::SchedulerWorkerDelegate> base/task_scheduler/scheduler_single_thread_task_runner_manager.cc:490:13 (content_browsertests+0x4bd4867) #5 base::internal::SchedulerSingleThreadTaskRunnerManager::CreateSingleThreadTaskRunnerWithTraits(base::TaskTraits const&, base::SingleThreadTaskRunnerThreadMode) base/task_scheduler/scheduler_single_thread_task_runner_manager.cc:426 (content_browsertests+0x4bd4867) #6 base::internal::TaskSchedulerImpl::CreateSingleThreadTaskRunnerWithTraits(base::TaskTraits const&, base::SingleThreadTaskRunnerThreadMode) base/task_scheduler/task_scheduler_impl.cc:155:8 (content_browsertests+0x4bcb27c) #7 base::CreateSingleThreadTaskRunnerWithTraits(base::TaskTraits const&, base::SingleThreadTaskRunnerThreadMode) base/task_scheduler/post_task.cc:115:40 (content_browsertests+0x4bc979c) #8 Create base/task_scheduler/lazy_task_runner.cc:54:10 (content_browsertests+0x4bc8e41) #9 base::internal::LazyTaskRunner<base::SingleThreadTaskRunner, false>::CreateRaw(void*) base/task_scheduler/lazy_task_runner.cc:72 (content_browsertests+0x4bc8e41) #10 GetOrCreateLazyPointer<base::SingleThreadTaskRunner> base/lazy_instance_helpers.h:83:48 (content_browsertests+0x4bc8daa) #11 base::internal::LazyTaskRunner<base::SingleThreadTaskRunner, false>::Get() base/task_scheduler/lazy_task_runner.cc:92 (content_browsertests+0x4bc8daa) #12 GetProcessLauncherTaskRunner content/browser/child_process_launcher_helper.cc:219:31 (content_browsertests+0x3d83cdc) #13 content::internal::ChildProcessLauncherHelper::StartLaunchOnClientThread() content/browser/child_process_launcher_helper.cc:99 (content_browsertests+0x3d83cdc) #14 content::ChildProcessLauncher::ChildProcessLauncher(std::__1::unique_ptr<content::SandboxedProcessLauncherDelegate, std::__1::default_delete<content::SandboxedProcessLauncherDelegate> >, std::__1::unique_ptr<base::CommandLine, std::__1::default_delete<base::CommandLine> >, int, content::ChildProcessLauncher::Client*, std::__1::unique_ptr<mojo::edk::OutgoingBrokerClientInvitation, std::__1::default_delete<mojo::edk::OutgoingBrokerClientInvitation> >, base::RepeatingCallback<void (std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&)> const&, bool) content/browser/child_process_launcher.cc:48:12 (content_browsertests+0x3d8315e) #15 content::BrowserChildProcessHostImpl::Launch(std::__1::unique_ptr<content::SandboxedProcessLauncherDelegate, std::__1::default_delete<content::SandboxedProcessLauncherDelegate> >, std::__1::unique_ptr<base::CommandLine, std::__1::default_delete<base::CommandLine> >, bool) content/browser/browser_child_process_host_impl.cc:268:28 (content_browsertests+0x3d222dc) #16 content::GpuProcessHost::LaunchGpuProcess() content/browser/gpu/gpu_process_host.cc:1327:13 (content_browsertests+0x3f54ced) ...
,
Apr 25 2018
I'm pretty sure accessing Content<Foo>Clients are allowed on any thread. I believe they are supposed to be set before any threads are started, and never changed again. Individual methods should documented thread safety concerns. There are probably lots of example where it's not called on the UI/Main thread, eg: https://cs.chromium.org/chromium/src/content/browser/browser_child_process_host_impl.cc?rcl=c474af563adbcba225b58f7d674a271aa8cbc2e2&l=166 Umm.. so should these be suppressed somehow?
,
Apr 25 2018
Given the model is to set a client first, and then allow reading it from multiple threads. This issue shows we are violating that. The read has already happened on another thread, and then a write is happening. DownloadContentTest probably need to move the client initialization into SetUp() or something. Calling SetBrowserClientForTesting() in the test body is too late.
,
May 28 2018
This is still causing redness https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20TSan%20Tests/22040
,
Jun 4 2018
Ping @ jianli, still causing redness (and keeping sheriffs busy) https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20TSan%20Tests/22334
,
Jun 4 2018
,
Jun 5 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/de94d59196994afbb5f26865535b7154163fa53a commit de94d59196994afbb5f26865535b7154163fa53a Author: Jian Li <jianli@chromium.org> Date: Tue Jun 05 20:16:50 2018 Move the browser client initialization for MHTML download tests to SetUp This is to avoid potential data race issues in tests. Bug: 836442 , 837104 Change-Id: Ic46e2834e1a50fa3644e6fe5ec0d4ef874e0b0a0 Reviewed-on: https://chromium-review.googlesource.com/1086156 Reviewed-by: Min Qin <qinmin@chromium.org> Commit-Queue: Jian Li <jianli@chromium.org> Cr-Commit-Position: refs/heads/master@{#564638} [modify] https://crrev.com/de94d59196994afbb5f26865535b7154163fa53a/content/browser/download/download_browsertest.cc
,
Jun 5 2018
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by thestig@chromium.org
, Apr 24 2018