DescriptionRevert of Pass task runners to AudioManager constructor. (patchset #50 id:980001 of https://codereview.chromium.org/1806313003/ )
Reason for revert:
Broke TSan: https://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20TSan%20Tests/builds/19362
WARNING: ThreadSanitizer: data race (pid=25863)
Write of size 8 at 0x0000081b8060 by main thread:
#0 media::AudioManagerDeleter::operator()(media::AudioManager const*) const media/audio/audio_manager.cc:264:20 (content_browsertests+0x000001e77078)
#1 reset buildtools/third_party/libc++/trunk/include/memory:2735:13 (content_browsertests+0x000004e5f2af)
#2 ~unique_ptr buildtools/third_party/libc++/trunk/include/memory:2703 (content_browsertests+0x000004e5f2af)
#3 content::BrowserMainLoop::~BrowserMainLoop() content/browser/browser_main_loop.cc:428 (content_browsertests+0x000004e5f2af)
#4 content::BrowserMainLoop::~BrowserMainLoop() content/browser/browser_main_loop.cc:424:37 (content_browsertests+0x000004e5f779)
#5 operator() buildtools/third_party/libc++/trunk/include/memory:2529:13 (content_browsertests+0x000004e68120)
#6 reset buildtools/third_party/libc++/trunk/include/memory:2735 (content_browsertests+0x000004e68120)
#7 content::BrowserMainRunnerImpl::Shutdown() content/browser/browser_main_runner.cc:223 (content_browsertests+0x000004e68120)
#8 ShellBrowserMain(content::MainFunctionParams const&, std::__1::unique_ptr<content::BrowserMainRunner, std::__1::default_delete<content::BrowserMainRunner> > const&) content/shell/browser/shell_browser_main.cc:33:3 (content_browsertests+0x000000c97ce7)
#9 content::ShellMainDelegate::RunProcess(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, content::MainFunctionParams const&) content/shell/app/shell_main_delegate.cc:288:16 (content_browsertests+0x000000c8bec4)
#10 content::RunNamedProcessTypeMain(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, content::MainFunctionParams const&, content::ContentMainDelegate*) content/app/content_main_runner.cc:368:25 (content_browsertests+0x000005558812)
#11 content::ContentMainRunnerImpl::Run() content/app/content_main_runner.cc:742:12 (content_browsertests+0x000005559511)
#12 content::ContentMain(content::ContentMainParams const&) content/app/content_main.cc:20:15 (content_browsertests+0x00000555797e)
#13 content::BrowserTestBase::SetUp() content/public/test/browser_test_base.cc:287:3 (content_browsertests+0x000000c55601)
#14 content::ContentBrowserTest::SetUp() content/public/test/content_browser_test.cc:92:3 (content_browsertests+0x000000c5e8e6)
#15 SetUp content/browser/renderer_host/render_widget_host_view_browsertest.cc:221:5 (content_browsertests+0x000000739618)
#16 content::(anonymous namespace)::CompositingRenderWidgetHostViewBrowserTestTabCapture::SetUp() content/browser/renderer_host/render_widget_host_view_browsertest.cc:403 (content_browsertests+0x000000739618)
#17 HandleExceptionsInMethodIfSupported<testing::Test, void> testing/gtest/src/gtest.cc:2458:12 (content_browsertests+0x000000f3168d)
#18 testing::Test::Run() testing/gtest/src/gtest.cc:2470 (content_browsertests+0x000000f3168d)
#19 testing::TestInfo::Run() testing/gtest/src/gtest.cc:2656:5 (content_browsertests+0x000000f32833)
#20 testing::TestCase::Run() testing/gtest/src/gtest.cc:2774:5 (content_browsertests+0x000000f33118)
#21 testing::internal::UnitTestImpl::RunAllTests() testing/gtest/src/gtest.cc:4647:11 (content_browsertests+0x000000f3c562)
#22 HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> testing/gtest/src/gtest.cc:2458:12 (content_browsertests+0x000000f3bfc6)
#23 testing::UnitTest::Run() testing/gtest/src/gtest.cc:4255 (content_browsertests+0x000000f3bfc6)
#24 RUN_ALL_TESTS testing/gtest/include/gtest/gtest.h:2237:10 (content_browsertests+0x000000d209eb)
#25 base::TestSuite::Run() base/test/test_suite.cc:230 (content_browsertests+0x000000d209eb)
#26 content::ContentTestLauncherDelegate::RunTestSuite(int, char**) content/test/content_test_launcher.cc:105:12 (content_browsertests+0x000000c677db)
#27 content::LaunchTests(content::TestLauncherDelegate*, int, int, char**) content/public/test/test_launcher.cc:517:12 (content_browsertests+0x000000cfb2fc)
#28 main content/test/content_test_launcher.cc:131:10 (content_browsertests+0x000000c67762)
Previous read of size 8 at 0x0000081b8060 by thread T15:
#0 media::(anonymous namespace)::AudioManagerHelper::UpdateLastAudioThreadTimeTick() media/audio/audio_manager.cc:179:5 (content_browsertests+0x000001e77986)
#1 Run<> base/bind_internal.h:181:12 (content_browsertests+0x000001e77df2)
#2 MakeItSo<media::(anonymous namespace)::AudioManagerHelper *> base/bind_internal.h:321 (content_browsertests+0x000001e77df2)
#3 base::internal::Invoker<base::IndexSequence<0ul>, base::internal::BindState<base::internal::RunnableAdapter<void (media::(anonymous namespace)::AudioManagerHelper::*)()>, void (media::(anonymous namespace)::AudioManagerHelper*), base::internal::UnretainedWrapper<media::(anonymous namespace)::AudioManagerHelper> >, base::internal::InvokeHelper<false, void, base::internal::RunnableAdapter<void (media::(anonymous namespace)::AudioManagerHelper::*)()> >, void ()>::Run(base::internal::BindStateBase*) base/bind_internal.h:372 (content_browsertests+0x000001e77df2)
#4 Run base/callback.h:397:12 (content_browsertests+0x00000111aa7d)
#5 base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask const&) base/debug/task_annotator.cc:51 (content_browsertests+0x00000111aa7d)
#6 base::MessageLoop::RunTask(base::PendingTask const&) base/message_loop/message_loop.cc:479:3 (content_browsertests+0x0000010958e6)
#7 base::MessageLoop::DeferOrRunPendingTask(base::PendingTask const&) base/message_loop/message_loop.cc:488:5 (content_browsertests+0x000001095ecd)
#8 base::MessageLoop::DoDelayedWork(base::TimeTicks*) base/message_loop/message_loop.cc:638:10 (content_browsertests+0x00000109662a)
#9 base::MessagePumpDefault::Run(base::MessagePump::Delegate*) base/message_loop/message_pump_default.cc:37:17 (content_browsertests+0x000001099875)
#10 base::MessageLoop::RunHandler() base/message_loop/message_loop.cc:443:3 (content_browsertests+0x00000109528b)
#11 base::RunLoop::Run() base/run_loop.cc:35:3 (content_browsertests+0x0000010b5cc6)
#12 base::MessageLoop::Run() base/message_loop/message_loop.cc:295:3 (content_browsertests+0x000001094a95)
#13 base::Thread::Run(base::MessageLoop*) base/threading/thread.cc:202:3 (content_browsertests+0x0000010de279)
#14 base::Thread::ThreadMain() base/threading/thread.cc:254:3 (content_browsertests+0x0000010de449)
#15 base::(anonymous namespace)::ThreadFunc(void*) base/threading/platform_thread_posix.cc:70:3 (content_browsertests+0x0000010d88bd)
Location is global 'media::(anonymous namespace)::g_last_created' of size 8 at 0x0000081b8060 (content_browsertests+0x0000081b8060)
Thread T15 'AudioThread' (tid=25884, running) created by main thread at:
#0 pthread_create <null> (content_browsertests+0x0000004a15c5)
#1 base::(anonymous namespace)::CreateThread(unsigned long, bool, base::PlatformThread::Delegate*, base::PlatformThreadHandle*, base::ThreadPriority) base/threading/platform_thread_posix.cc:109:13 (content_browsertests+0x0000010d860a)
#2 base::PlatformThread::CreateWithPriority(unsigned long, base::PlatformThread::Delegate*, base::PlatformThreadHandle*, base::ThreadPriority) base/threading/platform_thread_posix.cc:190:10 (content_browsertests+0x0000010d8515)
#3 base::Thread::StartWithOptions(base::Thread::Options const&) base/threading/thread.cc:116:10 (content_browsertests+0x0000010ddfa0)
#4 base::Thread::Start() base/threading/thread.cc:86:10 (content_browsertests+0x0000010ddde4)
#5 content::BrowserMainLoop::CreateAudioManager() content/browser/browser_main_loop.cc:1468:3 (content_browsertests+0x000004e665c6)
#6 content::BrowserMainLoop::BrowserThreadsStarted() content/browser/browser_main_loop.cc:1228:5 (content_browsertests+0x000004e633b5)
#7 Run<> base/bind_internal.h:181:12 (content_browsertests+0x000004e67612)
#8 MakeItSo<content::BrowserMainLoop *> base/bind_internal.h:313 (content_browsertests+0x000004e67612)
#9 base::internal::Invoker<base::IndexSequence<0ul>, base::internal::BindState<base::internal::RunnableAdapter<int (content::BrowserMainLoop::*)()>, int (content::BrowserMainLoop*), base::internal::UnretainedWrapper<content::BrowserMainLoop> >, base::internal::InvokeHelper<false, int, base::internal::RunnableAdapter<int (content::BrowserMainLoop::*)()> >, int ()>::Run(base::internal::BindStateBase*) base/bind_internal.h:372 (content_browsertests+0x000004e67612)
#10 Run base/callback.h:397:12 (content_browsertests+0x0000050dd941)
#11 content::StartupTaskRunner::RunAllTasksNow() content/browser/startup_task_runner.cc:45 (content_browsertests+0x0000050dd941)
#12 content::BrowserMainLoop::CreateStartupTasks() content/browser/browser_main_loop.cc:803:3 (content_browsertests+0x000004e622db)
#13 content::BrowserMainRunnerImpl::Initialize(content::MainFunctionParams const&) content/browser/browser_main_runner.cc:139:5 (content_browsertests+0x000004e67c63)
#14 ShellBrowserMain(content::MainFunctionParams const&, std::__1::unique_ptr<content::BrowserMainRunner, std::__1::default_delete<content::BrowserMainRunner> > const&) content/shell/browser/shell_browser_main.cc:23:19 (content_browsertests+0x000000c97ca7)
#15 content::ShellMainDelegate::RunProcess(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, content::MainFunctionParams const&) content/shell/app/shell_main_delegate.cc:288:16 (content_browsertests+0x000000c8bec4)
#16 content::RunNamedProcessTypeMain(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, content::MainFunctionParams const&, content::ContentMainDelegate*) content/app/content_main_runner.cc:368:25 (content_browsertests+0x000005558812)
#17 content::ContentMainRunnerImpl::Run() content/app/content_main_runner.cc:742:12 (content_browsertests+0x000005559511)
#18 content::ContentMain(content::ContentMainParams const&) content/app/content_main.cc:20:15 (content_browsertests+0x00000555797e)
#19 content::BrowserTestBase::SetUp() content/public/test/browser_test_base.cc:287:3 (content_browsertests+0x000000c55601)
#20 content::ContentBrowserTest::SetUp() content/public/test/content_browser_test.cc:92:3 (content_browsertests+0x000000c5e8e6)
#21 SetUp content/browser/renderer_host/render_widget_host_view_browsertest.cc:221:5 (content_browsertests+0x000000739618)
#22 content::(anonymous namespace)::CompositingRenderWidgetHostViewBrowserTestTabCapture::SetUp() content/browser/renderer_host/render_widget_host_view_browsertest.cc:403 (content_browsertests+0x000000739618)
#23 HandleExceptionsInMethodIfSupported<testing::Test, void> testing/gtest/src/gtest.cc:2458:12 (content_browsertests+0x000000f3168d)
#24 testing::Test::Run() testing/gtest/src/gtest.cc:2470 (content_browsertests+0x000000f3168d)
#25 testing::TestInfo::Run() testing/gtest/src/gtest.cc:2656:5 (content_browsertests+0x000000f32833)
#26 testing::TestCase::Run() testing/gtest/src/gtest.cc:2774:5 (content_browsertests+0x000000f33118)
#27 testing::internal::UnitTestImpl::RunAllTests() testing/gtest/src/gtest.cc:4647:11 (content_browsertests+0x000000f3c562)
#28 HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> testing/gtest/src/gtest.cc:2458:12 (content_browsertests+0x000000f3bfc6)
#29 testing::UnitTest::Run() testing/gtest/src/gtest.cc:4255 (content_browsertests+0x000000f3bfc6)
#30 RUN_ALL_TESTS testing/gtest/include/gtest/gtest.h:2237:10 (content_browsertests+0x000000d209eb)
#31 base::TestSuite::Run() base/test/test_suite.cc:230 (content_browsertests+0x000000d209eb)
#32 content::ContentTestLauncherDelegate::RunTestSuite(int, char**) content/test/content_test_launcher.cc:105:12 (content_browsertests+0x000000c677db)
#33 content::LaunchTests(content::TestLauncherDelegate*, int, int, char**) content/public/test/test_launcher.cc:517:12 (content_browsertests+0x000000cfb2fc)
#34 main content/test/content_test_launcher.cc:131:10 (content_browsertests+0x000000c67762)
SUMMARY: ThreadSanitizer: data race media/audio/audio_manager.cc:264:20 in media::AudioManagerDeleter::operator()(media::AudioManager const*) const
Original issue's description:
> Pass task runners to AudioManager constructor.
>
> This patch replaces the internal AudioManagerBase::audio_thread with
> an external task runner. The old implementation made it harder to test
> and override the internal audio thread.
>
> AudioManagerBase::Shutdown had to complete on the audio thread before
> the AudioManager instance could be deleted. It relied on Thread::Stop
> on the main thread to wait for the audio thread to shutdown. A subclass
> using a different thread shared with other modules could not do that.
>
> Passing a task runner into AudioManager and making GetTaskRunner
> non-virtual ensures that the task runner will not change for the
> lifetime of AudioManager instance.
>
> BUG=594234
>
> Committed: https://crrev.com/e05981bd06b4694b426990acd41be8ab6d9a1659
> Cr-Commit-Position: refs/heads/master@{#388048}
TBR=dalecurtis@chromium.org,tommi@chromium.org,xhwang@chromium.org,rkc@chromium.org,jam@chromium.org,ckehoe@chromium.org,alokp@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=594234
Committed: https://crrev.com/78655ccbbd314f7ceb4133cd0d395dd35502fdf9
Cr-Commit-Position: refs/heads/master@{#388096}
Patch Set 1 #Messages
Total messages: 6 (2 generated)
|