No SurfaceClient During Surface::ActivateFrame |
||||
Issue descriptionNew crash I haven't seen before showing up in the mash_browser_tests. First run with it: https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.fyi%2FMojo_ChromiumOS%2F20165%2F%2B%2Frecipes%2Fsteps%2Fmash_browser_tests%2F0%2Fstdout [0720/144731.179062:FATAL:surface.cc(237)] Check failed: surface_client_. #0 0x0000037a660c base::debug::StackTrace::StackTrace() #1 0x0000037be7a1 logging::LogMessage::~LogMessage() #2 0x0000055b7e22 cc::Surface::ActivateFrame() #3 0x0000055b873a cc::Surface::ActivatePendingFrame() #4 0x0000055b9f20 cc::SurfaceDependencyDeadline::OnBeginFrame() #5 0x000004e4a2a2 cc::ExternalBeginFrameSource::OnBeginFrame() #6 0x00000563bfe6 viz::PrimaryBeginFrameSource::OnBeginFrame() #7 0x000004e497ac cc::DelayBasedBeginFrameSource::OnTimerTick() #8 0x000004e4a9c1 cc::DelayBasedTimeSource::OnTimerTick() #9 0x000000764f9f _ZN4base8internal7InvokerINS0_9BindStateIMN12_GLOBAL__N_116SimpleHttpServer10ConnectionEFvvEJNS_7WeakPtrIS5_EEEEEFvvEE7RunOnceEPNS0_13BindStateBaseE #10 0x000000764f9f _ZN4base8internal7InvokerINS0_9BindStateIMN12_GLOBAL__N_116SimpleHttpServer10ConnectionEFvvEJNS_7WeakPtrIS5_EEEEEFvvEE7RunOnceEPNS0_13BindStateBaseE #11 0x000003853e50 base::debug::TaskAnnotator::RunTask() #12 0x0000037c5e55 base::MessageLoop::RunTask() #13 0x0000037c610b base::MessageLoop::DeferOrRunPendingTask() #14 0x0000037c66d3 base::MessageLoop::DoDelayedWork() #15 0x0000037c88ad base::MessagePumpLibevent::Run() #16 0x0000037c5a8b base::MessageLoop::Run() #17 0x0000037f032a base::RunLoop::Run() #18 0x000003797442 (anonymous namespace)::StartEmbeddedService() #19 0x0000024f872d _ZN4base8internal7InvokerINS0_9BindStateIPFvN4mojo16InterfaceRequestIN5blink5mojom19KeyboardLockServiceEEEEJEEES9_E3RunEPNS0_13BindStateBaseEOS8_ #20 0x0000021ed322 service_manager::RunStandaloneService() #21 0x00000379706e RunMashBrowserTests() #22 0x000003796ec7 main #23 0x7f99ba98af45 __libc_start_main #24 0x00000062a29e <unknown> Any ideas? If not I can check when I get back.
,
Jul 20 2017
Possible test case: 1. A CompositorFrame without any dependencies is submitted to B and B activates 2. A CompositorFrame that depends on B is submitted to A and activates. 3. A CompositorFrame that depends on C is submitted to B. 4. B's CompositorFrameSinkSupport is destroyed. 5. B is kept alive by A. 6. A deadline hits and B's pending frame activates...|surface_client_| is invalid. One easy solution is to just add a guard to check if |surface_client_| is set. However, we would display a stale frame. A better solution is maybe to update the frame independent of the CompositorFrameSinkSupport. This would require that surface references and surface synchronization is moved entirely into the surfaces layer...which is probably the right architecture anyway.
,
Jul 22 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8a4c9feabb78ad6cd14ef59897f150c7017f7cfa commit 8a4c9feabb78ad6cd14ef59897f150c7017f7cfa Author: Fady Samuel <fsamuel@chromium.org> Date: Sat Jul 22 03:05:39 2017 viz: Fix CompositorFrame activation after FrameSink destruction The following scenario could crash Viz prior to this patch: 1. A CompositorFrame without any dependencies is submitted to B and B activates 2. A CompositorFrame that depends on B is submitted to A and activates. 3. A CompositorFrame that depends on C is submitted to B. 4. B's CompositorFrameSinkSupport is destroyed. 5. B is kept alive by A. 6. A deadline hits and B's pending frame activates...|surface_client_| is invalid and CRASH. This CL adds a unit test for this scenario, and removes a DCHECK in Surface::ActivateFrame and instead guards access to surface_client_. This is not the ideal solution because the parent surface will refer to a stale child until the next display frame is generated (this activation does not trigger a display frame), but it's better than a crash. Bug: 747164 TBR: weiliangc@chromium.org Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel Change-Id: I3b5491d4c8843f1009f0ecbb7a085c38946a7c04 Reviewed-on: https://chromium-review.googlesource.com/580747 Commit-Queue: Fady Samuel <fsamuel@chromium.org> Reviewed-by: Fady Samuel <fsamuel@chromium.org> Cr-Commit-Position: refs/heads/master@{#488846} [modify] https://crrev.com/8a4c9feabb78ad6cd14ef59897f150c7017f7cfa/cc/surfaces/surface.cc [modify] https://crrev.com/8a4c9feabb78ad6cd14ef59897f150c7017f7cfa/components/viz/service/frame_sinks/surface_synchronization_unittest.cc
,
Jul 28 2017
Since the immediate bug is fixed I'm marking as FIXED>
,
Feb 26 2018
|
||||
►
Sign in to add a comment |
||||
Comment 1 by fsam...@chromium.org
, Jul 20 2017Status: Assigned (was: Untriaged)