DisplayLinkMac::GetAllDisplayLinks is being called on the wrong thread? |
||
Issue description
This function does the following:
DisplayLinkMac::DisplayLinkMap& DisplayLinkMac::GetAllDisplayLinks() {
DCHECK_EQ(base::ThreadTaskRunnerHandle::Get(), GetMainThreadTaskRunner());
static base::NoDestructor<DisplayLinkMac::DisplayLinkMap> all_display_links;
return *all_display_links;
}
and then there's:
scoped_refptr<base::SingleThreadTaskRunner>
DisplayLinkMac::GetMainThreadTaskRunner() {
static scoped_refptr<base::SingleThreadTaskRunner> task_runner =
base::ThreadTaskRunnerHandle::Get();
return task_runner;
}
sometimes when running the full suite the DCHECK_EQ in GetAllDisplayLinks will fail. Unfortunately thread handles are fairly opaque :(.
,
Sep 19
I figured this one out: the static variable in DisplayLinkMac::GetMainThreadTaskRunner() is not reset between tests, so the first test in a suite runs properly, and then in the second test that static is already initialized - C++ has no way to know it should be reinitialized. That leads to a DCHECK because ThreadTaskRunnerHandle's state *is* reset. That reset is accomplished via ~ViewsTestBase, which calls ~ScopedTaskEnvironment, which destroys message_loop_. I guess we should probably just delete this DCHECK, or have it DCHECK instead that we're on the main thread by PlatformThreadRef. Over to ccameron@ :)
,
Sep 25
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/61e34c2a024cb9849ac2112c233b44fe4ed384e0 commit 61e34c2a024cb9849ac2112c233b44fe4ed384e0 Author: Christopher Cameron <ccameron@chromium.org> Date: Tue Sep 25 22:37:15 2018 Replace DCHECK in DisplayLinkMac with comments Move some of the static methods in DisplayLinkMac to be anonymous namespace methods in the cc file, and add comments clarifying their usage. Remove the DCHECK ensuring that the initially-captured main thread task runner be equal to the current main thread task runner, and add a comment indicating that this may not always be the case. Bug: 885329 Change-Id: Idfdeb3ba1d20840161f26669e19bdee42aa155c5 Reviewed-on: https://chromium-review.googlesource.com/1234229 Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org> Commit-Queue: ccameron <ccameron@chromium.org> Cr-Commit-Position: refs/heads/master@{#594129} [modify] https://crrev.com/61e34c2a024cb9849ac2112c233b44fe4ed384e0/ui/accelerated_widget_mac/display_link_mac.cc [modify] https://crrev.com/61e34c2a024cb9849ac2112c233b44fe4ed384e0/ui/accelerated_widget_mac/display_link_mac.h |
||
►
Sign in to add a comment |
||
Comment 1 by ellyjo...@chromium.org
, Sep 19(lldb) bt * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BREAKPOINT (code=EXC_I386_BPT, subcode=0x0) frame #0: 0x000000010ef1d921 libbase.dylib`base::debug::BreakDebugger() at debugger_posix.cc:272 [opt] frame #1: 0x000000010ee1aa95 libbase.dylib`logging::LogMessage::~LogMessage(this=0x00007ffeefbfe300) at logging.cc:856 [opt] * frame #2: 0x0000000116c5dc6a libaccelerated_widget_mac.dylib`ui::DisplayLinkMac::GetAllDisplayLinks() at display_link_mac.cc:197 [opt] frame #3: 0x0000000116c5d864 libaccelerated_widget_mac.dylib`ui::DisplayLinkMac::GetForDisplay(display_id=<unavailable>) at display_link_mac.cc:44 [opt] frame #4: 0x000000010eb98546 libviews.dylib`views::BridgedNativeWidgetHostImpl::OnWindowDisplayChanged(this=0x000000011f241ed0, new_display=0x00007ffeefbfe730) at bridged_native_widget_host_impl.mm:652 [opt] frame #5: 0x000000010eae5681 libviews.dylib`views::BridgedNativeWidgetImpl::InitCompositorView() [inlined] views::BridgedNativeWidgetImpl::UpdateWindowDisplay(this=<unavailable>) at bridged_native_widget.mm:1277 [opt] frame #6: 0x000000010eae5652 libviews.dylib`views::BridgedNativeWidgetImpl::InitCompositorView(this=0x000000011f2dad30) at bridged_native_widget.mm:916 [opt] frame #7: 0x000000010eb96b1a libviews.dylib`views::BridgedNativeWidgetHostImpl::CreateCompositor(this=0x000000011f241ed0, params=<unavailable>) at bridged_native_widget_host_impl.mm:272 [opt] frame #8: 0x000000010eb76304 libviews.dylib`views::NativeWidgetMac::InitNativeWidget(this=0x000000011f2b0290, params=0x00007ffeefbfeb88) at native_widget_mac.mm:158 [opt] frame #9: 0x000000010eb7d62a libviews.dylib`views::Widget::Init(this=0x000000011f2b3130, in_params=<unavailable>) at widget.cc:342 [opt] frame #10: 0x0000000101ec0cb9 unit_tests`OmniboxViewViewsTest::SetUp(this=0x000000011f844000) at omnibox_view_views_unittest.cc:280 [opt] frame #11: 0x00000001020d33cd unit_tests`testing::Test::Run() [inlined] void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(object=<unavailable>, method=11 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00)(), char const*) at gtest.cc:0 [opt] frame #12: 0x00000001020d33bf unit_tests`testing::Test::Run(this=0x000000011f844000) at gtest.cc:2518 [opt] frame #13: 0x00000001020d4050 unit_tests`testing::TestInfo::Run(this=0x0000000121cd5900) at gtest.cc:2698 [opt] frame #14: 0x00000001020d45c7 unit_tests`testing::TestCase::Run(this=0x0000000121cd57f0) at gtest.cc:2816 [opt] frame #15: 0x00000001020e0557 unit_tests`testing::internal::UnitTestImpl::RunAllTests(this=0x000000011f601bc0) at gtest.cc:5182 [opt] frame #16: 0x00000001020e01cd unit_tests`testing::UnitTest::Run() [inlined] bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(object=0x000000011f601bc0, method=00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00)(), char const*) at gtest.cc:0 [opt] frame #17: 0x00000001020e01c0 unit_tests`testing::UnitTest::Run(this=<unavailable>) at gtest.cc:4791 [opt] frame #18: 0x0000000102844a76 unit_tests`base::TestSuite::Run() [inlined] RUN_ALL_TESTS() at gtest.h:2333 [opt] frame #19: 0x0000000102844a69 unit_tests`base::TestSuite::Run(this=0x0000000121d11690) at test_suite.cc:295 [opt] frame #20: 0x000000010285443e unit_tests`base::(anonymous namespace)::LaunchUnitTestsInternal(base::OnceCallback<int ()>, unsigned long, int, bool, base::OnceCallback<void ()>) [inlined] base::OnceCallback<int ()>::Run(this=<unavailable>) && at callback.h:99 [opt] frame #21: 0x00000001028543dc unit_tests`base::(anonymous namespace)::LaunchUnitTestsInternal(run_test_suite=base::RunTestSuiteCallback @ 0x00007ffeefbff720, parallel_jobs=1, default_batch_limit=10, use_job_objects=true, gtest_init=base::OnceClosure @ 0x00007ffeefbff718)>, unsigned long, int, bool, base::OnceCallback<void ()>) at unit_test_launcher.cc:225 [opt] frame #22: 0x0000000102854291 unit_tests`base::LaunchUnitTests(argc=2, argv=<unavailable>, run_test_suite=<unavailable>)>) at unit_test_launcher.cc:576 [opt] frame #23: 0x000000010283543b unit_tests`main(argc=2, argv=0x00007ffeefbff980) at run_all_unittests.cc:30 [opt] frame #24: 0x00007fff50e48015 libdyld.dylib`start + 1