New issue
Advanced search Search tips

Issue 807781 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug

Blocking:
issue 678687



Sign in to add a comment

mash_browser_tests flaky DCHECK gfx::ClientNativePixmapFactory::GetInstance() in ui::Service::OnStart()

Project Member Reported by jamescook@chromium.org, Jan 31 2018

Issue description

This seems to happen randomly during test startup, about 0.5% of the time. It succeeds on retry, which is why we haven't seen test suite failures on the FYI bot. I'm nervous about adding more mash_browser_tests to the main waterfall until we sort out this flake.

https://chromium-swarm.appspot.com/task?id=3b6661e0d27ae310&refresh=10&show_raw=1&wide_logs=true

[ RUN      ] FileSystemApiTestForRequestFileSystem.Background
[17804:17858:0131/111245.974357:FATAL:service.cc(266)] Check failed: gfx::ClientNativePixmapFactory::GetInstance(). 
#0 0x000003e9513c base::debug::StackTrace::StackTrace()
#1 0x000003eaf34c logging::LogMessage::~LogMessage()
#2 0x000002d62c5c ui::Service::OnStart()
#3 0x0000048b9840 service_manager::ServiceContext::OnStart()
#4 0x00000526cc87 service_manager::mojom::ServiceStubDispatch::AcceptWithResponder()
#5 0x0000048ba106 service_manager::mojom::ServiceStub<>::AcceptWithResponder()
#6 0x0000052438fc mojo::InterfaceEndpointClient::HandleValidatedMessage()
#7 0x00000525aa46 mojo::FilterChain::Accept()
#8 0x000005244c95 mojo::InterfaceEndpointClient::HandleIncomingMessage()
#9 0x00000524d99d mojo::internal::MultiplexRouter::ProcessIncomingMessage()
#10 0x00000524cedc mojo::internal::MultiplexRouter::Accept()
#11 0x00000525aa46 mojo::FilterChain::Accept()
#12 0x0000052429e5 mojo::Connector::ReadSingleMessage()
#13 0x000005243301 mojo::Connector::ReadAllAvailableMessages()
#14 0x0000052431a9 mojo::Connector::OnHandleReadyInternal()
#15 0x000002519327 mojo::SimpleWatcher::DiscardReadyState()
#16 0x000005260ca6 mojo::SimpleWatcher::OnHandleReady()
#17 0x0000052611c1 _ZN4base8internal7InvokerINS0_9BindStateIMN4mojo13SimpleWatcherEFvijRKNS3_18HandleSignalsStateEEJNS_7WeakPtrIS4_EEijS5_EEEFvvEE7RunImplIRKS9_RKNSt3__15tupleIJSB_ijS5_EEEJLm0ELm1ELm2ELm3EEEEvOT_OT0_NSI_16integer_sequenceImJXspT1_EEEE
#18 0x000003e958f5 base::debug::TaskAnnotator::RunTask()
#19 0x000003f4d0b9 base::internal::IncomingTaskQueue::RunTask()
#20 0x000003eb6d0b base::MessageLoop::RunTask()

https://cs.chromium.org/chromium/src/services/ui/service.cc?type=cs&q=ui::Service::OnStart&sq=package:chromium&l=266

This CL changed factory initialization but it's 10 months old: https://codereview.chromium.org/2803523002

I'm guessing this is a side effect of putting the ash service and the ui service in the same process -- maybe both threads are trying to access the same g_instance of the factory?

 
I can repro this locally with a linux-chromeos debug build if I make this code change:

void ClientNativePixmapFactory::SetInstance(
    ClientNativePixmapFactory* instance) {
  DCHECK(!g_instance);
  DCHECK(instance);
  // ****
  LOG(ERROR) << "JAMES SetInstance " << instance;
  base::debug::StackTrace().Print();
  // ****
  g_instance = instance;
}

This changes the timing enough to cause the failure.

It looks like SetInstance() is being called by the ash service on one thread:
#2 0x7f34d9c3015d gfx::ClientNativePixmapFactory::SetInstance()
#3 0x7f34d16ab475 aura::Env::Init()
#4 0x7f34d16ab311 aura::Env::CreateInstance()
#5 0x7f34cb388a3a views::AuraInit::Init()
#6 0x7f34cb3887a9 views::AuraInit::Create()
#7 0x7f34cd1a5d48 ash::WindowManagerService::OnStart()

But GetInstance() is checked by the ui service on a different thread, with the stack in the bug description.

This is --mash right? So WS thread initializes ozone? If so: 

https://cs.chromium.org/chromium/src/services/ui/service.cc?rcl=a51901e5154ce660fc9e706d6a2b63202a949325&l=260 and aura::Env::Init would both need a gate to skip setting the instance if it's already set?


Owner: jamescook@chromium.org
Status: Started (was: Untriaged)
I have it working with a single thread-safe owner of the singleton object. Waiting for tryjobs on https://chromium-review.googlesource.com/#/c/chromium/src/+/897987
Cc: spang@chromium.org
+spang FYI - comment #1 has a code tweak that repros the browser_tests --mash DCHECK.

Project Member

Comment 5 by bugdroid1@chromium.org, Feb 13 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/66c7d1df05655b74be91a1d17a958795f027fb09

commit 66c7d1df05655b74be91a1d17a958795f027fb09
Author: James Cook <jamescook@chromium.org>
Date: Tue Feb 13 06:21:38 2018

ozone: Fix mash_browser_tests DCHECKs in gfx::ClientNativePixmapFactory

In chrome --mash and browser_tests --mash the ash service and window
service run in the same process but on different threads. There's a race
between the ash service creation of gfx::ClientNativePixmapFactory and
the window service's use of it.

Change the gfx::ClientNativePixmapFactory to be owned by a thread-safe
base::Singleton so that it gets created once by whichever service needs
it first.

Also eliminate an unused reference to the factory in BrowserMainLoop.

Bug:  807781 
Test: mash_browser_tests

Change-Id: I7b77c3d6ad957c87ed947caa703b7f65161ebb04
Reviewed-on: https://chromium-review.googlesource.com/897987
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536267}
[modify] https://crrev.com/66c7d1df05655b74be91a1d17a958795f027fb09/content/browser/browser_main_loop.h
[modify] https://crrev.com/66c7d1df05655b74be91a1d17a958795f027fb09/content/renderer/renderer_main.cc
[modify] https://crrev.com/66c7d1df05655b74be91a1d17a958795f027fb09/services/ui/service.cc
[modify] https://crrev.com/66c7d1df05655b74be91a1d17a958795f027fb09/services/ui/service.h
[modify] https://crrev.com/66c7d1df05655b74be91a1d17a958795f027fb09/ui/aura/env.cc
[modify] https://crrev.com/66c7d1df05655b74be91a1d17a958795f027fb09/ui/aura/env.h
[modify] https://crrev.com/66c7d1df05655b74be91a1d17a958795f027fb09/ui/ozone/gl/gl_image_ozone_native_pixmap_unittest.cc
[modify] https://crrev.com/66c7d1df05655b74be91a1d17a958795f027fb09/ui/ozone/public/client_native_pixmap_factory_ozone.cc
[modify] https://crrev.com/66c7d1df05655b74be91a1d17a958795f027fb09/ui/ozone/public/client_native_pixmap_factory_ozone.h

Status: Fixed (was: Started)
mash_browser_tests are not flaky anymore.

https://ci.chromium.org/buildbot/chromium.fyi/Mojo%20ChromiumOS/
Components: -Internals>MUS Internals>Services>WindowService
Project Member

Comment 8 by bugdroid1@chromium.org, Mar 8 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21

commit 4d2bcd1e0e5782e44a71bf3f1d89560fe587be21
Author: Michael Spang <spang@chromium.org>
Date: Thu Mar 08 02:26:47 2018

Remove the global ClientNativePixmapFactory

Make each buffer management object responsible for holding a
ClientNativePixmapFactory, removing the need for a global one. This
simplifies a number of initialization and ownership scenarios.

This makes it impossible for MUS to race to initialize the object, since
each object that needs it will have its own (sharing the factory globally
is not actually necessary).

To make this work, this moves GpuMemoryBufferImpl factory to a new object
GpuMemoryBufferSupport, since the majority of objects which should own a
ClientNativePixmapFactory used it via GpuMemoryBufferImpl::Create().

Bug:  807781 
Test: browser_tests

Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Icde20b247905555e80767b8c3eb77af6be9bbce1
Reviewed-on: https://chromium-review.googlesource.com/899949
Commit-Queue: Michael Spang <spang@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: David Reveman <reveman@chromium.org>
Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541696}
[modify] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/chrome/browser/chromeos/arc/screen_capture/arc_screen_capture_session.cc
[modify] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/chrome/browser/chromeos/arc/screen_capture/arc_screen_capture_session.h
[modify] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/components/exo/display.cc
[modify] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/components/exo/display.h
[modify] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/components/exo/shared_memory.cc
[modify] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/components/viz/host/server_gpu_memory_buffer_manager.cc
[modify] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/components/viz/host/server_gpu_memory_buffer_manager.h
[modify] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/components/viz/host/server_gpu_memory_buffer_manager_unittest.cc
[modify] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/components/viz/service/display_embedder/in_process_gpu_memory_buffer_manager.cc
[modify] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/components/viz/service/display_embedder/in_process_gpu_memory_buffer_manager.h
[modify] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/content/browser/gpu/browser_gpu_memory_buffer_manager.cc
[modify] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/content/browser/gpu/browser_gpu_memory_buffer_manager.h
[modify] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/content/browser/gpu/gpu_client.cc
[modify] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/content/browser/gpu/gpu_internals_ui.cc
[modify] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/content/browser/renderer_host/render_message_filter.cc
[modify] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/content/renderer/render_thread_impl_discardable_memory_browsertest.cc
[modify] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/content/renderer/renderer_main.cc
[modify] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/gpu/BUILD.gn
[modify] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/gpu/ipc/client/BUILD.gn
[delete] https://crrev.com/848031b9423f78b09f7557aed2e22f48e7824deb/gpu/ipc/client/gpu_memory_buffer_impl.cc
[modify] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/gpu/ipc/common/BUILD.gn
[add] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/gpu/ipc/common/gpu_memory_buffer_impl.cc
[rename] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/gpu/ipc/common/gpu_memory_buffer_impl.h
[rename] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/gpu/ipc/common/gpu_memory_buffer_impl_android_hardware_buffer.cc
[rename] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/gpu/ipc/common/gpu_memory_buffer_impl_android_hardware_buffer.h
[rename] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/gpu/ipc/common/gpu_memory_buffer_impl_android_hardware_buffer_unittest.cc
[rename] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/gpu/ipc/common/gpu_memory_buffer_impl_dxgi.cc
[rename] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/gpu/ipc/common/gpu_memory_buffer_impl_dxgi.h
[rename] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/gpu/ipc/common/gpu_memory_buffer_impl_dxgi_unittest.cc
[rename] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/gpu/ipc/common/gpu_memory_buffer_impl_io_surface.cc
[rename] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/gpu/ipc/common/gpu_memory_buffer_impl_io_surface.h
[rename] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/gpu/ipc/common/gpu_memory_buffer_impl_io_surface_unittest.cc
[rename] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/gpu/ipc/common/gpu_memory_buffer_impl_native_pixmap.cc
[rename] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/gpu/ipc/common/gpu_memory_buffer_impl_native_pixmap.h
[rename] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/gpu/ipc/common/gpu_memory_buffer_impl_native_pixmap_unittest.cc
[rename] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/gpu/ipc/common/gpu_memory_buffer_impl_shared_memory.cc
[rename] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/gpu/ipc/common/gpu_memory_buffer_impl_shared_memory.h
[rename] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/gpu/ipc/common/gpu_memory_buffer_impl_shared_memory_unittest.cc
[rename] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/gpu/ipc/common/gpu_memory_buffer_impl_test_template.h
[modify] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/gpu/ipc/common/gpu_memory_buffer_support.cc
[modify] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/gpu/ipc/common/gpu_memory_buffer_support.h
[modify] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/gpu/ipc/host/gpu_memory_buffer_support.cc
[modify] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/gpu/ipc/host/gpu_memory_buffer_support.h
[modify] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/gpu/ipc/service/gpu_memory_buffer_factory_test_template.h
[modify] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/services/ui/public/cpp/gpu/client_gpu_memory_buffer_manager.cc
[modify] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/services/ui/public/cpp/gpu/client_gpu_memory_buffer_manager.h
[modify] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/services/ui/service.cc
[modify] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/services/ui/ws/gpu_host.cc
[modify] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/ui/aura/env.cc
[modify] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/ui/aura/env.h
[modify] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/ui/gfx/client_native_pixmap_factory.cc
[modify] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/ui/gfx/client_native_pixmap_factory.h
[modify] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/ui/ozone/gl/gl_image_ozone_native_pixmap_unittest.cc
[modify] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/ui/ozone/public/client_native_pixmap_factory_ozone.cc
[modify] https://crrev.com/4d2bcd1e0e5782e44a71bf3f1d89560fe587be21/ui/ozone/public/client_native_pixmap_factory_ozone.h

Sign in to add a comment