Chrome crashes during shutdown with --enable-viz |
|||||
Issue descriptionChrome Version: ToT OS: Linux Steps to reproduce: (1) Run Linux desktop Chrome with --enable-viz. (2) Close the initial browser window. (3) Crash! Chrome shouldn't crash on shutdown. This can also happens if you open a second Chrome window and close one of the two open windows. I think this is the same race as crbug.com/646982 . There is normally a sync IPC when destroying CommandBufferProxyImpl that ensures the correct destruction order of XWindows. With --enable-viz there is no such sync IPC for the display compositor anymore.
,
Oct 31 2017
,
Oct 31 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/afd9794a34e3e11e065398deb9c6170fff66aca3 commit afd9794a34e3e11e065398deb9c6170fff66aca3 Author: Makoto Shimazu <shimazu@chromium.org> Date: Tue Oct 31 03:36:00 2017 Revert "glx: Fix a crash during teardown." This reverts commit c74bc22553928bd0c920a25118308f56c5cac432. Reason for revert: Several layout tests are failing on Mac. This is a log of one of the failing bots: https://storage.googleapis.com/chromium-layout-test-archives/Mac10_10_Tests/25548/layout-test-results/results.html Original change's description: > glx: Fix a crash during teardown. > > Two notable changes: > . Do not immediately destroy the GLContextGLX when MakeCurrent() fails. > This follows the pattern in GLContextEGL, where MakeCurrent() does > not immediately destroy the context on failure. > . Notify TextureManager about lost context early during tear down. This > is necessary because FramebufferManager is destroyed first, and > during its destruction, some gpu::gles2::Texture objects can be > destroyed. But TextureManager is destroyed later, and is notified of > the lost context only during destruction. As a result, Texture > destruction does glDeleteTextures() while there's no context, causing > a crash. > > BUG= 777594 > > 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: I664e43c297cded17c223d3ee1ac4e32ca28ffd42 > Reviewed-on: https://chromium-review.googlesource.com/734945 > Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org> > Reviewed-by: Antoine Labour <piman@chromium.org> > Cr-Commit-Position: refs/heads/master@{#512651} TBR=sadrul@chromium.org,piman@chromium.org Change-Id: I4eea9ab43cb390057ba95282585625118975f699 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 777594 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 Reviewed-on: https://chromium-review.googlesource.com/745685 Reviewed-by: Makoto Shimazu <shimazu@chromium.org> Commit-Queue: Makoto Shimazu <shimazu@chromium.org> Cr-Commit-Position: refs/heads/master@{#512733} [modify] https://crrev.com/afd9794a34e3e11e065398deb9c6170fff66aca3/gpu/command_buffer/service/context_group.cc [modify] https://crrev.com/afd9794a34e3e11e065398deb9c6170fff66aca3/gpu/command_buffer/service/framebuffer_manager_unittest.cc [modify] https://crrev.com/afd9794a34e3e11e065398deb9c6170fff66aca3/gpu/command_buffer/service/gles2_cmd_decoder.cc [modify] https://crrev.com/afd9794a34e3e11e065398deb9c6170fff66aca3/gpu/command_buffer/service/gles2_cmd_decoder_unittest_context_lost.cc [modify] https://crrev.com/afd9794a34e3e11e065398deb9c6170fff66aca3/gpu/command_buffer/service/texture_manager.cc [modify] https://crrev.com/afd9794a34e3e11e065398deb9c6170fff66aca3/gpu/command_buffer/service/texture_manager.h [modify] https://crrev.com/afd9794a34e3e11e065398deb9c6170fff66aca3/gpu/command_buffer/service/texture_manager_unittest.cc [modify] https://crrev.com/afd9794a34e3e11e065398deb9c6170fff66aca3/ui/gl/BUILD.gn [modify] https://crrev.com/afd9794a34e3e11e065398deb9c6170fff66aca3/ui/gl/gl_context.h [modify] https://crrev.com/afd9794a34e3e11e065398deb9c6170fff66aca3/ui/gl/gl_context_glx.cc [delete] https://crrev.com/5dcaa25d8585ce209f32122a800a3cec648cea60/ui/gl/gl_context_glx_unittest.cc
,
Oct 31 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3f4dc7b2c38ce980afec699fc7827ba6e60643a7 commit 3f4dc7b2c38ce980afec699fc7827ba6e60643a7 Author: Sadrul Habib Chowdhury <sadrul@chromium.org> Date: Tue Oct 31 07:44:55 2017 Reland "glx: Fix a crash during teardown." This is a reland of c74bc22553928bd0c920a25118308f56c5cac432 Original change's description: > glx: Fix a crash during teardown. > > Two notable changes: > . Do not immediately destroy the GLContextGLX when MakeCurrent() fails. > This follows the pattern in GLContextEGL, where MakeCurrent() does > not immediately destroy the context on failure. > . Notify TextureManager about lost context early during tear down. This > is necessary because FramebufferManager is destroyed first, and > during its destruction, some gpu::gles2::Texture objects can be > destroyed. But TextureManager is destroyed later, and is notified of > the lost context only during destruction. As a result, Texture > destruction does glDeleteTextures() while there's no context, causing > a crash. > > BUG= 777594 > > 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: I664e43c297cded17c223d3ee1ac4e32ca28ffd42 > Reviewed-on: https://chromium-review.googlesource.com/734945 > Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org> > Reviewed-by: Antoine Labour <piman@chromium.org> > Cr-Commit-Position: refs/heads/master@{#512651} TBR=piman@, since the fix for the revert is a trivial nullcheck Bug: 777594 Change-Id: I0d02ef632e0754ab9cf398a349048108b1a05ed9 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 Reviewed-on: https://chromium-review.googlesource.com/746241 Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org> Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org> Cr-Commit-Position: refs/heads/master@{#512773} [modify] https://crrev.com/3f4dc7b2c38ce980afec699fc7827ba6e60643a7/gpu/command_buffer/service/context_group.cc [modify] https://crrev.com/3f4dc7b2c38ce980afec699fc7827ba6e60643a7/gpu/command_buffer/service/framebuffer_manager_unittest.cc [modify] https://crrev.com/3f4dc7b2c38ce980afec699fc7827ba6e60643a7/gpu/command_buffer/service/gles2_cmd_decoder.cc [modify] https://crrev.com/3f4dc7b2c38ce980afec699fc7827ba6e60643a7/gpu/command_buffer/service/gles2_cmd_decoder_unittest_context_lost.cc [modify] https://crrev.com/3f4dc7b2c38ce980afec699fc7827ba6e60643a7/gpu/command_buffer/service/texture_manager.cc [modify] https://crrev.com/3f4dc7b2c38ce980afec699fc7827ba6e60643a7/gpu/command_buffer/service/texture_manager.h [modify] https://crrev.com/3f4dc7b2c38ce980afec699fc7827ba6e60643a7/gpu/command_buffer/service/texture_manager_unittest.cc [modify] https://crrev.com/3f4dc7b2c38ce980afec699fc7827ba6e60643a7/ui/gl/BUILD.gn [modify] https://crrev.com/3f4dc7b2c38ce980afec699fc7827ba6e60643a7/ui/gl/gl_context.h [modify] https://crrev.com/3f4dc7b2c38ce980afec699fc7827ba6e60643a7/ui/gl/gl_context_glx.cc [add] https://crrev.com/3f4dc7b2c38ce980afec699fc7827ba6e60643a7/ui/gl/gl_context_glx_unittest.cc
,
Oct 31 2017
There are more outstanding crashes. Reopening.
,
Oct 31 2017
,
Oct 31 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5b7348a874c3008e3cba64bfd439e9a5a7bba47a commit 5b7348a874c3008e3cba64bfd439e9a5a7bba47a Author: Sadrul Habib Chowdhury <sadrul@chromium.org> Date: Tue Oct 31 20:16:57 2017 gpu: Invalidate the fence if context is lost. Make sure to invalidate the fence if context was lost, so that it doesn't attempt to make gl calls during tear down. BUG= 777594 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: I0722193e020fe01483d55e17d640a99061cdb7ea Reviewed-on: https://chromium-review.googlesource.com/747381 Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org> Reviewed-by: Antoine Labour <piman@chromium.org> Cr-Commit-Position: refs/heads/master@{#512929} [modify] https://crrev.com/5b7348a874c3008e3cba64bfd439e9a5a7bba47a/gpu/command_buffer/service/gles2_cmd_decoder_unittest_context_lost.cc [modify] https://crrev.com/5b7348a874c3008e3cba64bfd439e9a5a7bba47a/gpu/command_buffer/service/query_manager.cc
,
Nov 14 2017
Issue 646982 has been merged into this issue.
,
Jan 25 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/698da7e959bcea6758232ee56a298a52cd74ddd8 commit 698da7e959bcea6758232ee56a298a52cd74ddd8 Author: kylechar <kylechar@chromium.org> Date: Thu Jan 25 14:08:18 2018 viz: Sync destruction for RootCompositorFrameSinkImpl. For not OOP-D, when destroying a viz::Display there is a sync IPC in CommandBufferProxyImpl::DisconnectChannel() that ensures the GL context/surface that draw to the platform window are destroyed. The platform window is destroyed after that sync IPC which ensures the GPU doesn't try to swap to a buffer that doesn't exist. Currently for OOP-D, HostFrameSinkManager sends an async IPC to the GPU process asking it to destroy the viz::Display. The browser process destroys the platform window after sending the async IPC. The GPU may try to swap to a buffer that doesn't exist and error/hang/crash. Add a sync IPC, only used in the case of root CompositorFrameSinks, that guarantees the GL context/surface are destroyed. This eliminates a big source of GPU hangs/crashes with --enable-features=VizDisplayCompositor. Bug: 777594 Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel Change-Id: I70e9b031e80de233856194f1beaa9377f332c3c0 Reviewed-on: https://chromium-review.googlesource.com/874724 Commit-Queue: kylechar <kylechar@chromium.org> Reviewed-by: Tom Sepez <tsepez@chromium.org> Reviewed-by: Ken Rockot <rockot@chromium.org> Reviewed-by: Antoine Labour <piman@chromium.org> Cr-Commit-Position: refs/heads/master@{#531889} [modify] https://crrev.com/698da7e959bcea6758232ee56a298a52cd74ddd8/components/viz/host/host_frame_sink_manager.cc [modify] https://crrev.com/698da7e959bcea6758232ee56a298a52cd74ddd8/components/viz/service/frame_sinks/frame_sink_manager_impl.cc [modify] https://crrev.com/698da7e959bcea6758232ee56a298a52cd74ddd8/components/viz/service/frame_sinks/frame_sink_manager_impl.h [modify] https://crrev.com/698da7e959bcea6758232ee56a298a52cd74ddd8/components/viz/test/test_frame_sink_manager.h [modify] https://crrev.com/698da7e959bcea6758232ee56a298a52cd74ddd8/mojo/public/cpp/bindings/sync_call_restrictions.h [modify] https://crrev.com/698da7e959bcea6758232ee56a298a52cd74ddd8/services/viz/privileged/interfaces/compositing/frame_sink_manager.mojom
,
Feb 6 2018
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by bugdroid1@chromium.org
, Oct 30 2017