New issue
Advanced search Search tips

Issue 829484 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

SoftwareTextureLayerLoseFrameSinkTest.RunSingleThread_DelegatingRenderer TSan failures

Project Member Reported by kylec...@chromium.org, Apr 5 2018

Issue description

There are failures in cc_unittests SoftwareTextureLayerLoseFrameSinkTest.RunSingleThread_DelegatingRenderer when running with TSan on Linux. Example failure:

https://logs.chromium.org/v/?s=chromium%2Fbb%2Ftryserver.chromium.linux%2Flinux_chromium_tsan_rel_ng%2F281458%2F%2B%2Frecipes%2Fsteps%2Fcc_unittests__with_patch_%2F0%2Flogs%2FSoftwareTextureLayerLoseFrameSinkTest.RunSingleThread_DelegatingRenderer%2F0

It reproduces locally sometimes at ToT. I'm not sure why it's not causing more problems on the waterfall. The test was introduced in https://crrev.com/c/976694 and danakj is out so I'll investigate.


 
I believe the problem in SoftwareTextureLayerLoseFrameSinkTest is the following. Each time cc::Scheduler::OnBeginImplFrameDeadline() runs it advances the test with SoftwareTextureLayerLoseFrameSinkTest::DidCommitAndDrawFrame(). On the second call to  SoftwareTextureLayerLoseFrameSinkTest::DidCommitAndDrawFrame() it will destroy the TestLayerTreeFrameSink and replace it with a new one. If cc::Scheduler::OnBeginImplFrameDeadline() gets PostTasked to directly (I believe it comes from ScrollbarAnimationController) then everything works fine. 

However the TestLayerTreeFrameSink also has a DelayBasedBeginFrameSource, if that ticks and triggers cc::Scheduler::OnBeginImplFrameDeadline() then the DelayBasedBeginFrameSource is destroyed inside it's own callstack. There are a bunch of use-after-frees when this happens.

The test has to run very slow for the DelayBasedBeginFrameSource to fire which I guess is why the failure is so rare. It won't reproduce in ASan at ToT but after adding some log statements ASan is also slow enough to hit the same use-after-free.

piman do you know what should be controlling the OnBeginFrame() in these tests exactly?
 Issue 829923  has been merged into this issue.

Comment 3 by piman@chromium.org, Apr 6 2018

DelayBasedBeginFrameSource sounds like a bad idea for reproducible tests, generally, unfortunately it looks like the default.
But either way, I think if DelayBasedBeginFrameSource can get destroyed within its own callstack, this sounds like a potential problem in production, independently of the test, and has either to be fixed, or has to be resilient to that?

Comment 4 by piman@chromium.org, Apr 6 2018

Cc: enne@chromium.org
Cc: -hubbe@chromium.org
Labels: -Pri-1 Pri-3
Owner: danakj@chromium.org
I don't think this is an issue with production. The problem is that TestLayerTreeFrameSink is the whole compositing stack wrapped up into one object. SoftwareTextureLayerLoseFrameSinkTest tests LayerTreeFrameSink loss and destroys TestLayerTreeFrameSink. The display frame getting generated is the signal the test uses to destroy TestLayerTreeFrameSink, but that also destroys the Display/BeginFrameSource and causes use-after-free.

The test is disabled now so I'm downgrading the priority here and assigning back to danakj.
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 19 2018

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

commit 6dd12972bad7c598184a9d23935f467e76bad2d9
Author: danakj <danakj@chromium.org>
Date: Thu Apr 19 15:08:19 2018

cc: Fix flaky UAF in SoftwareTextureLayerLoseFrameSinkTest

The test removes the TestLayerTreeFrameSink from the LayerTreeHost,
which destroys its BeginFrameSource, Display, etc. However it was doing
this inside a method where the cc::Scheduler, BeginFrameSource, etc
are on the stack, so then it unwinds to a UAF scenario.

Fix this by PostTasking to a fresh stack for each step of the test so
the compositor is not on the stack when interacting with it.

R=kylechar@chromium.org

Bug:  829484 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I1868d3549c8c3a2b9a9612365bd21753ad713d8e
Reviewed-on: https://chromium-review.googlesource.com/1017769
Reviewed-by: kylechar <kylechar@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552019}
[modify] https://crrev.com/6dd12972bad7c598184a9d23935f467e76bad2d9/cc/layers/texture_layer_unittest.cc

Comment 7 by danakj@chromium.org, Apr 19 2018

Status: Fixed (was: Started)

Sign in to add a comment