New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 862409 link

Starred by 3 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Evaluate gpu related crashes from shortcut_viewer/tap_visualizer

Project Member Reported by sky@chromium.org, Jul 10

Issue description

There are a few graphics-related crashes in tap_visualizer and shortcut_viewer:

https://crash.corp.google.com/browse?q=product_name%3D%27Chrome_ChromeOS%27+AND+expanded_custom_data.ChromeCrashProto.ptype%3D%27shortcut_viewer_app%27
https://crash.corp.google.com/browse?q=product_name%3D%27Chrome_ChromeOS%27+AND+expanded_custom_data.ChromeCrashProto.ptype%3D%27tap_visualizer_app%27

These are the top signatures:
gpu::GpuChannelHost::Send
cc::mojo_embedder::AsyncLayerTreeFrameSink::SubmitCompositorFrame

tap_visualizer and shortcut_viewer were only recently moved out of process and converted to use aura-mus/views-mus. I'm not familiar enough with the gpu to know if crashes like this are to be expected, or indicate a bigger issue.

Gary, I'm not sure who would be best to look at this so I'm passing your way to assign appropriately.
 
Owner: fsam...@chromium.org
Looks like Gary is on vacation for two weeks. So assigning to Fady.
Looks like you didn't pass along "enable_surface_synchronization" in AsyncLayerTreeFrameSink?
AFAICT the apps (shortcut_viewer, tap_visualizer) are setting enable_surface_synchronization. The code for that is here: https://chromium.googlesource.com/chromium/src/+/master/ui/aura/mus/window_port_mus.cc#133 . Is there some where else it needs to be set?
The line that's crashing is this:

https://cs.chromium.org/chromium/src/cc/mojo_embedder/async_layer_tree_frame_sink.cc?q=async_layer_tree&sq=package:chromium&g=0&l=131

This code path can only be taken if surface synchronization is off. I don't know how we get into this state yet.
Ahh OK, those are surface invariants violations then...there are cases a device scale factor change or resize is happening and we're not allocating a new LocalSurfaceId!
I suspect ClientRoot [1] isn't doing the right thing if the device scale factor changes. That would explain the AsyncLayerTreeFrameSink crash. What about gpu::GpuChannelHost::Send ? https://crash.corp.google.com/browse?q=product_name%3D%27Chrome_ChromeOS%27+AND+expanded_custom_data.ChromeCrashProto.ptype%3D%27shortcut_viewer_app%27+AND+expanded_custom_data.ChromeCrashProto.magic_signature_1.name%3D%27gpu%3A%3AGpuChannelHost%3A%3ASend%27

I'll look into the device scale factor one.

[1] - https://chromium.googlesource.com/chromium/src/+/master/services/ui/ws2/client_root.cc
It looks like it's hanging. I'm not sure why. Maybe the gpu process failed? 
Status: Assigned (was: Untriaged)
For the GpuChannelHost::Send() case, it does look like it is caused by the gpu-process crash. For example: at the same time as the GpuChannelHost::Send() crash [1], there is a crash in the gpu-process for the same client too [2]. Similarly for another client, crash in shortcut-viewer [3] and gpu-process [4] at the same time. I didn't look any farther, but I suspect they all are the same.

gpu-process crash is recoverable though. The client should be able to recover if/when the gpu-process crashes.


[1] https://crash.corp.google.com/browse?q=ReportID%3D%27c45a1f2034169f62%27&stbtiq=&reportid=&index=0
[2] https://crash.corp.google.com/browse?q=ReportID%3D%27a238aac29ccdeadb%27&stbtiq=&reportid=&index=0
[3] https://crash.corp.google.com/browse?q=ReportID%3D%2733e4e1ac1e186a7d%27&stbtiq=&reportid=&index=0
[4] https://crash.corp.google.com/browse?q=ReportID%3D%27e9db65fe37f19007%27&stbtiq=&reportid=&index=0
It looks like CommandBufferProxyImpl::Send() calls GpuChannelHost::Send(), which has a call to WaitableEvent::Wait() in it. WaitableEvent::Wait() DCHECKs on success. I assume the wait is failing because the other end (gpu) is crashing. If that is in fact the cause, then it seems CommandBufferProxyImpl shouldn't use Wait(). Instead CommandBufferProxyImpl should use Wait() with an infinite timeout and deal with a false return. Does that sound right?
Project Member

Comment 12 by bugdroid1@chromium.org, Jul 17

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

commit b4e6da646f25cca291318e50e7aa9dd05173dd65
Author: Scott Violet <sky@chromium.org>
Date: Tue Jul 17 22:14:59 2018

chromeos: makes ScreenProviderObserver an associated interface

And changes ws2 to ensure client is notified when scale-factor changes.
The conversion from ScreenProviderObserver to associated interface is to
ensure the ScreenProviderObserver is notified *before* the bounds change
because of scale-factor. To do otherwise means the client might use the wrong
scale-factor.

BUG=862409
TEST=none

Change-Id: I78333dcabe3e4adb1e2091cba784d53d9c27be1a
Reviewed-on: https://chromium-review.googlesource.com/1138866
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575799}
[modify] https://crrev.com/b4e6da646f25cca291318e50e7aa9dd05173dd65/ash/app_launch_unittest.cc
[modify] https://crrev.com/b4e6da646f25cca291318e50e7aa9dd05173dd65/ash/display/window_tree_host_manager.cc
[modify] https://crrev.com/b4e6da646f25cca291318e50e7aa9dd05173dd65/content/renderer/mus/renderer_window_tree_client.cc
[modify] https://crrev.com/b4e6da646f25cca291318e50e7aa9dd05173dd65/content/renderer/mus/renderer_window_tree_client.h
[modify] https://crrev.com/b4e6da646f25cca291318e50e7aa9dd05173dd65/services/ui/public/interfaces/screen_provider.mojom
[modify] https://crrev.com/b4e6da646f25cca291318e50e7aa9dd05173dd65/services/ui/public/interfaces/window_tree.mojom
[modify] https://crrev.com/b4e6da646f25cca291318e50e7aa9dd05173dd65/services/ui/ws/test_utils.cc
[modify] https://crrev.com/b4e6da646f25cca291318e50e7aa9dd05173dd65/services/ui/ws/test_utils.h
[modify] https://crrev.com/b4e6da646f25cca291318e50e7aa9dd05173dd65/services/ui/ws/window_tree_client_unittest.cc
[modify] https://crrev.com/b4e6da646f25cca291318e50e7aa9dd05173dd65/services/ui/ws2/BUILD.gn
[modify] https://crrev.com/b4e6da646f25cca291318e50e7aa9dd05173dd65/services/ui/ws2/client_root.cc
[modify] https://crrev.com/b4e6da646f25cca291318e50e7aa9dd05173dd65/services/ui/ws2/client_root.h
[modify] https://crrev.com/b4e6da646f25cca291318e50e7aa9dd05173dd65/services/ui/ws2/screen_provider.cc
[modify] https://crrev.com/b4e6da646f25cca291318e50e7aa9dd05173dd65/services/ui/ws2/screen_provider.h
[modify] https://crrev.com/b4e6da646f25cca291318e50e7aa9dd05173dd65/services/ui/ws2/screen_provider_unittest.cc
[add] https://crrev.com/b4e6da646f25cca291318e50e7aa9dd05173dd65/services/ui/ws2/test_screen_provider_observer.cc
[add] https://crrev.com/b4e6da646f25cca291318e50e7aa9dd05173dd65/services/ui/ws2/test_screen_provider_observer.h
[modify] https://crrev.com/b4e6da646f25cca291318e50e7aa9dd05173dd65/services/ui/ws2/test_window_tree_client.cc
[modify] https://crrev.com/b4e6da646f25cca291318e50e7aa9dd05173dd65/services/ui/ws2/test_window_tree_client.h
[modify] https://crrev.com/b4e6da646f25cca291318e50e7aa9dd05173dd65/services/ui/ws2/window_service.cc
[modify] https://crrev.com/b4e6da646f25cca291318e50e7aa9dd05173dd65/services/ui/ws2/window_service.h
[modify] https://crrev.com/b4e6da646f25cca291318e50e7aa9dd05173dd65/services/ui/ws2/window_service_test_setup.h
[modify] https://crrev.com/b4e6da646f25cca291318e50e7aa9dd05173dd65/services/ui/ws2/window_tree_binding.cc
[modify] https://crrev.com/b4e6da646f25cca291318e50e7aa9dd05173dd65/services/ui/ws2/window_tree_binding.h
[modify] https://crrev.com/b4e6da646f25cca291318e50e7aa9dd05173dd65/services/ui/ws2/window_tree_unittest.cc
[modify] https://crrev.com/b4e6da646f25cca291318e50e7aa9dd05173dd65/ui/aura/mus/window_tree_client.cc
[modify] https://crrev.com/b4e6da646f25cca291318e50e7aa9dd05173dd65/ui/aura/mus/window_tree_client.h
[modify] https://crrev.com/b4e6da646f25cca291318e50e7aa9dd05173dd65/ui/aura/mus/window_tree_client_delegate.h
[modify] https://crrev.com/b4e6da646f25cca291318e50e7aa9dd05173dd65/ui/views/mus/mus_client.cc
[modify] https://crrev.com/b4e6da646f25cca291318e50e7aa9dd05173dd65/ui/views/mus/mus_client.h
[modify] https://crrev.com/b4e6da646f25cca291318e50e7aa9dd05173dd65/ui/views/mus/screen_mus.cc
[modify] https://crrev.com/b4e6da646f25cca291318e50e7aa9dd05173dd65/ui/views/mus/screen_mus.h

Project Member

Comment 13 by bugdroid1@chromium.org, Jul 18

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

commit bcd4dffd735a04da55c91ebe7f1959ddb8be36fc
Author: Yutaka Hirano <yhirano@chromium.org>
Date: Wed Jul 18 07:16:35 2018

Revert "chromeos: makes ScreenProviderObserver an associated interface"

This reverts commit b4e6da646f25cca291318e50e7aa9dd05173dd65.

Reason for revert: Broke PolicyDisplayRotationDefault/DisplayRotationBootTest.Reboot/*

Original change's description:
> chromeos: makes ScreenProviderObserver an associated interface
> 
> And changes ws2 to ensure client is notified when scale-factor changes.
> The conversion from ScreenProviderObserver to associated interface is to
> ensure the ScreenProviderObserver is notified *before* the bounds change
> because of scale-factor. To do otherwise means the client might use the wrong
> scale-factor.
> 
> BUG=862409
> TEST=none
> 
> Change-Id: I78333dcabe3e4adb1e2091cba784d53d9c27be1a
> Reviewed-on: https://chromium-review.googlesource.com/1138866
> Reviewed-by: Tom Sepez <tsepez@chromium.org>
> Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
> Commit-Queue: Scott Violet <sky@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#575799}

TBR=sadrul@chromium.org,sky@chromium.org,tsepez@chromium.org

Change-Id: Iccd11134913aa340630f422d471f9047661d36a4
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 862409,  864902 
Reviewed-on: https://chromium-review.googlesource.com/1141584
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575964}
[modify] https://crrev.com/bcd4dffd735a04da55c91ebe7f1959ddb8be36fc/ash/app_launch_unittest.cc
[modify] https://crrev.com/bcd4dffd735a04da55c91ebe7f1959ddb8be36fc/ash/display/window_tree_host_manager.cc
[modify] https://crrev.com/bcd4dffd735a04da55c91ebe7f1959ddb8be36fc/content/renderer/mus/renderer_window_tree_client.cc
[modify] https://crrev.com/bcd4dffd735a04da55c91ebe7f1959ddb8be36fc/content/renderer/mus/renderer_window_tree_client.h
[modify] https://crrev.com/bcd4dffd735a04da55c91ebe7f1959ddb8be36fc/services/ui/public/interfaces/screen_provider.mojom
[modify] https://crrev.com/bcd4dffd735a04da55c91ebe7f1959ddb8be36fc/services/ui/public/interfaces/window_tree.mojom
[modify] https://crrev.com/bcd4dffd735a04da55c91ebe7f1959ddb8be36fc/services/ui/ws/test_utils.cc
[modify] https://crrev.com/bcd4dffd735a04da55c91ebe7f1959ddb8be36fc/services/ui/ws/test_utils.h
[modify] https://crrev.com/bcd4dffd735a04da55c91ebe7f1959ddb8be36fc/services/ui/ws/window_tree_client_unittest.cc
[modify] https://crrev.com/bcd4dffd735a04da55c91ebe7f1959ddb8be36fc/services/ui/ws2/BUILD.gn
[modify] https://crrev.com/bcd4dffd735a04da55c91ebe7f1959ddb8be36fc/services/ui/ws2/client_root.cc
[modify] https://crrev.com/bcd4dffd735a04da55c91ebe7f1959ddb8be36fc/services/ui/ws2/client_root.h
[modify] https://crrev.com/bcd4dffd735a04da55c91ebe7f1959ddb8be36fc/services/ui/ws2/screen_provider.cc
[modify] https://crrev.com/bcd4dffd735a04da55c91ebe7f1959ddb8be36fc/services/ui/ws2/screen_provider.h
[modify] https://crrev.com/bcd4dffd735a04da55c91ebe7f1959ddb8be36fc/services/ui/ws2/screen_provider_unittest.cc
[delete] https://crrev.com/57186f5dff9d6a5215f6604ba4c897f919039ff2/services/ui/ws2/test_screen_provider_observer.cc
[delete] https://crrev.com/57186f5dff9d6a5215f6604ba4c897f919039ff2/services/ui/ws2/test_screen_provider_observer.h
[modify] https://crrev.com/bcd4dffd735a04da55c91ebe7f1959ddb8be36fc/services/ui/ws2/test_window_tree_client.cc
[modify] https://crrev.com/bcd4dffd735a04da55c91ebe7f1959ddb8be36fc/services/ui/ws2/test_window_tree_client.h
[modify] https://crrev.com/bcd4dffd735a04da55c91ebe7f1959ddb8be36fc/services/ui/ws2/window_service.cc
[modify] https://crrev.com/bcd4dffd735a04da55c91ebe7f1959ddb8be36fc/services/ui/ws2/window_service.h
[modify] https://crrev.com/bcd4dffd735a04da55c91ebe7f1959ddb8be36fc/services/ui/ws2/window_service_test_setup.h
[modify] https://crrev.com/bcd4dffd735a04da55c91ebe7f1959ddb8be36fc/services/ui/ws2/window_tree_binding.cc
[modify] https://crrev.com/bcd4dffd735a04da55c91ebe7f1959ddb8be36fc/services/ui/ws2/window_tree_binding.h
[modify] https://crrev.com/bcd4dffd735a04da55c91ebe7f1959ddb8be36fc/services/ui/ws2/window_tree_unittest.cc
[modify] https://crrev.com/bcd4dffd735a04da55c91ebe7f1959ddb8be36fc/ui/aura/mus/window_tree_client.cc
[modify] https://crrev.com/bcd4dffd735a04da55c91ebe7f1959ddb8be36fc/ui/aura/mus/window_tree_client.h
[modify] https://crrev.com/bcd4dffd735a04da55c91ebe7f1959ddb8be36fc/ui/aura/mus/window_tree_client_delegate.h
[modify] https://crrev.com/bcd4dffd735a04da55c91ebe7f1959ddb8be36fc/ui/views/mus/mus_client.cc
[modify] https://crrev.com/bcd4dffd735a04da55c91ebe7f1959ddb8be36fc/ui/views/mus/mus_client.h
[modify] https://crrev.com/bcd4dffd735a04da55c91ebe7f1959ddb8be36fc/ui/views/mus/screen_mus.cc
[modify] https://crrev.com/bcd4dffd735a04da55c91ebe7f1959ddb8be36fc/ui/views/mus/screen_mus.h

PolicyDisplayRotationDefault/DisplayRotationBootTest.Reboot is still failing:
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/28304
Project Member

Comment 15 by bugdroid1@chromium.org, Jul 18

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

commit 353af842027a6793e828f07c6c9554c714f1248b
Author: Scott Violet <sky@chromium.org>
Date: Wed Jul 18 21:34:42 2018

reland: chromeos: makes ScreenProviderObserver an associated interface

This is a reland of https://chromium-review.googlesource.com/c/chromium/src/+/1138866 .
Diff between patchsets 2 and 3 to see the diff.

And changes ws2 to ensure client is notified when scale-factor changes.
The conversion from ScreenProviderObserver to associated interface is to
ensure the ScreenProviderObserver is notified *before* the bounds change
because of scale-factor. To do otherwise means the client might use the wrong
scale-factor.

BUG=862409
TEST=none
TBR=tsepez@chromium.org, sadrul@chromium.org

Change-Id: I4a82d78cf880935d0c740ae3b98bd01c7eb61227
Reviewed-on: https://chromium-review.googlesource.com/1142185
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576222}
[modify] https://crrev.com/353af842027a6793e828f07c6c9554c714f1248b/ash/app_launch_unittest.cc
[modify] https://crrev.com/353af842027a6793e828f07c6c9554c714f1248b/ash/display/window_tree_host_manager.cc
[modify] https://crrev.com/353af842027a6793e828f07c6c9554c714f1248b/ash/ws/window_service_owner.h
[modify] https://crrev.com/353af842027a6793e828f07c6c9554c714f1248b/content/renderer/mus/renderer_window_tree_client.cc
[modify] https://crrev.com/353af842027a6793e828f07c6c9554c714f1248b/content/renderer/mus/renderer_window_tree_client.h
[modify] https://crrev.com/353af842027a6793e828f07c6c9554c714f1248b/services/ui/public/interfaces/screen_provider.mojom
[modify] https://crrev.com/353af842027a6793e828f07c6c9554c714f1248b/services/ui/public/interfaces/window_tree.mojom
[modify] https://crrev.com/353af842027a6793e828f07c6c9554c714f1248b/services/ui/ws/test_utils.cc
[modify] https://crrev.com/353af842027a6793e828f07c6c9554c714f1248b/services/ui/ws/test_utils.h
[modify] https://crrev.com/353af842027a6793e828f07c6c9554c714f1248b/services/ui/ws/window_tree_client_unittest.cc
[modify] https://crrev.com/353af842027a6793e828f07c6c9554c714f1248b/services/ui/ws2/BUILD.gn
[modify] https://crrev.com/353af842027a6793e828f07c6c9554c714f1248b/services/ui/ws2/client_root.cc
[modify] https://crrev.com/353af842027a6793e828f07c6c9554c714f1248b/services/ui/ws2/client_root.h
[modify] https://crrev.com/353af842027a6793e828f07c6c9554c714f1248b/services/ui/ws2/screen_provider.cc
[modify] https://crrev.com/353af842027a6793e828f07c6c9554c714f1248b/services/ui/ws2/screen_provider.h
[modify] https://crrev.com/353af842027a6793e828f07c6c9554c714f1248b/services/ui/ws2/screen_provider_unittest.cc
[add] https://crrev.com/353af842027a6793e828f07c6c9554c714f1248b/services/ui/ws2/test_screen_provider_observer.cc
[add] https://crrev.com/353af842027a6793e828f07c6c9554c714f1248b/services/ui/ws2/test_screen_provider_observer.h
[modify] https://crrev.com/353af842027a6793e828f07c6c9554c714f1248b/services/ui/ws2/test_window_tree_client.cc
[modify] https://crrev.com/353af842027a6793e828f07c6c9554c714f1248b/services/ui/ws2/test_window_tree_client.h
[modify] https://crrev.com/353af842027a6793e828f07c6c9554c714f1248b/services/ui/ws2/window_service.cc
[modify] https://crrev.com/353af842027a6793e828f07c6c9554c714f1248b/services/ui/ws2/window_service.h
[modify] https://crrev.com/353af842027a6793e828f07c6c9554c714f1248b/services/ui/ws2/window_service_test_setup.h
[modify] https://crrev.com/353af842027a6793e828f07c6c9554c714f1248b/services/ui/ws2/window_tree_binding.cc
[modify] https://crrev.com/353af842027a6793e828f07c6c9554c714f1248b/services/ui/ws2/window_tree_binding.h
[modify] https://crrev.com/353af842027a6793e828f07c6c9554c714f1248b/services/ui/ws2/window_tree_unittest.cc
[modify] https://crrev.com/353af842027a6793e828f07c6c9554c714f1248b/ui/aura/mus/window_tree_client.cc
[modify] https://crrev.com/353af842027a6793e828f07c6c9554c714f1248b/ui/aura/mus/window_tree_client.h
[modify] https://crrev.com/353af842027a6793e828f07c6c9554c714f1248b/ui/aura/mus/window_tree_client_delegate.h
[modify] https://crrev.com/353af842027a6793e828f07c6c9554c714f1248b/ui/views/mus/mus_client.cc
[modify] https://crrev.com/353af842027a6793e828f07c6c9554c714f1248b/ui/views/mus/mus_client.h
[modify] https://crrev.com/353af842027a6793e828f07c6c9554c714f1248b/ui/views/mus/screen_mus.cc
[modify] https://crrev.com/353af842027a6793e828f07c6c9554c714f1248b/ui/views/mus/screen_mus.h

Fady, do you have any ideas on the Gpu related crashes? See comments 10 and 11.
Owner: sadrul@chromium.org
To be honest, I don't understand the GPU crashes. sadrul@ or kylechar@ might? It's really hard to tell from the call stack. Passing to sadrul@ for thoughts.
Issue 841907 has been merged into this issue.
> I assume the wait is failing because the other end (gpu) is crashing.

The crash seems to always happen in pthread_cond_wait(). I don't think that necessarily means the gpu process has crashed? Is the gpu-process crashing that often anyway?

It is possible that MusContextFactory is doing something wrong (MusContextFactory::CreateLayerTreeFrameSink() is always in the callstack). I started looking at that, but haven't gone very far unfortunately.
One experimental change to try would be https://chromium-review.googlesource.com/c/chromium/src/+/1263277 But it's difficult to tell without reproing the failure.
I don't see very many GPU-related crashes in tap_visualizer or shortcut_viewer in M70 and M71. See links in the original description.

Maybe something got fixed? Are other processes crashing?

Labels: -Pri-1 Pri-2
[GPU Triage Council]

Sign in to add a comment