Crash in mozilla::layers::ShadowLayerForwarder::UseTextures

RESOLVED FIXED in Firefox 50

Status

()

defect
--
critical
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: marcia, Assigned: nical)

Tracking

({crash})

Trunk
mozilla51
Unspecified
Windows 10
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox48 wontfix, firefox49 wontfix, firefox-esr45 affected, firefox50+ fixed, firefox51 fixed)

Details

(Whiteboard: [gfx-noted], crash signature)

Attachments

(1 attachment, 1 obsolete attachment)

This bug was filed from the Socorro interface and is 
report bp-432a6aee-5fd3-4619-9097-8a6802160802.
=============================================================

New crash showing up on the Nightly radar: http://bit.ly/2ay3Aft. Appears to have started using 20160731030203. All Windows crashes with highest concentration of Windows 10.

Some of the URLs are videos.

ni on nsilva for possible ideas since he appears to have been working in that code.
Flags: needinfo?(nical.bugzilla)
Excellent. This is basically bug 1286437 with a more interesting stack trace.
Assignee: nobody → nical.bugzilla
Flags: needinfo?(nical.bugzilla)
Not asking review yet because I haven't verified that the patch fixes the issue. I have only been able to reproduce the crash on one laptop and only with an optimized build, I'll test the patch on that configuration tomorrow.
Whiteboard: [gfx-noted]
Comment on attachment 8777397 [details] [diff] [review]
Support switching a shared buffer provider to a new and/or incompatible PCompositorBridge

Review of attachment 8777397 [details] [diff] [review]:
-----------------------------------------------------------------

Unfortunately I am only able to reproduce the bug with a download nightly build, not with my own builds. That said, this patch makes it impossible to run into the assertion of the crash (incompatible channels when updating a canvas), because it checks that the channels are compatible before sending and tries to do something sensible (copy the data into something compatible with the new channel) in case it they are not.
Attachment #8777397 - Flags: review?(ethlin)
(In reply to Nicolas Silva [:nical] from comment #3)
> Comment on attachment 8777397 [details] [diff] [review]
> Support switching a shared buffer provider to a new and/or incompatible
> PCompositorBridge
> 
> Review of attachment 8777397 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Unfortunately I am only able to reproduce the bug with a download nightly
> build, not with my own builds. That said, this patch makes it impossible to
> run into the assertion of the crash (incompatible channels when updating a
> canvas), because it checks that the channels are compatible before sending
> and tries to do something sensible (copy the data into something compatible
> with the new channel) in case it they are not.

I can reproduce this bug with my own build by dragging a canvas tab to another window with e10s off. The behavior is similar to bug 1250954.
Comment on attachment 8777397 [details] [diff] [review]
Support switching a shared buffer provider to a new and/or incompatible PCompositorBridge

Review of attachment 8777397 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/PersistentBufferProvider.cpp
@@ +147,5 @@
> +    // The forwarder should not change most of the time.
> +    return true;
> +  }
> +
> +  if (mFwd->AsTextureForwarder() == aFwd->AsTextureForwarder() ||

Should this be '!='?
(In reply to Ethan Lin[:ethlin] from comment #5)
> Should this be '!='?

oh my. Yes it should!
Comment on attachment 8777397 [details] [diff] [review]
Support switching a shared buffer provider to a new and/or incompatible PCompositorBridge

Cancelling the review, there is another adjustement I need t make to this patch regarding the activity tracker. I am also able to reproduce the bug with e10s disabled with my own builds now (thanks for the tip!).
Attachment #8777397 - Flags: review?(ethlin)
I have been able to verify that this patch fixes the problem.
Attachment #8777397 - Attachment is obsolete: true
Attachment #8777779 - Flags: review?(ethlin)
Comment on attachment 8777779 [details] [diff] [review]
Support switching a shared buffer provider to a new and/or incompatible PCompositorBridge

Review of attachment 8777779 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/PersistentBufferProvider.cpp
@@ +170,5 @@
> +        BackendSelector::Canvas,
> +        TextureFlags::DEFAULT,
> +        TextureAllocationFlags::ALLOC_DEFAULT
> +      );
> +

I think we should have 'MOZ_ASSERT(newTexture)' here.
Attachment #8777779 - Flags: review?(ethlin) → review+
Pushed by nsilva@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d59fd1ff4bec
Make it possible to change a PersistentBufferProvider's Forwarder without crashing. r=ethlin
[Tracking Requested - why for this release]: #6 in the top crasher list for Firefox 50.
https://hg.mozilla.org/mozilla-central/rev/d59fd1ff4bec
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Hello Nicolas, could you please request uplift to Aurora50? Thanks!
Flags: needinfo?(nical.bugzilla)
Comment on attachment 8777779 [details] [diff] [review]
Support switching a shared buffer provider to a new and/or incompatible PCompositorBridge

Approval Request Comment
[Feature/regressing bug #]:
[User impact if declined]: Crashes
[Describe test coverage new/current, TreeHerder]: None.
[Risks and why]: Low risk, has been baking on central for a while.
[String/UUID change made/needed]: None.
Flags: needinfo?(nical.bugzilla)
Attachment #8777779 - Flags: approval-mozilla-aurora?
Comment on attachment 8777779 [details] [diff] [review]
Support switching a shared buffer provider to a new and/or incompatible PCompositorBridge

Crash fix, the data from Nightly showed this still happened on 08-16 (even with the fix) but I still think this is worth uplifting to Aurora50.
Attachment #8777779 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Crash volume for signature 'mozilla::layers::ShadowLayerForwarder::UseTextures':
 - nightly (version 51): 46 crashes from 2016-08-01.
 - aurora  (version 50): 414 crashes from 2016-08-01.
 - beta    (version 49): 3 crashes from 2016-08-02.
 - release (version 48): 2 crashes from 2016-07-25.
 - esr     (version 45): 1 crash from 2016-05-02.

Crash volume on the last weeks (Week N is from 08-22 to 08-28):
            W. N-1  W. N-2  W. N-3
 - nightly      10       3      26
 - aurora      115     241      48
 - beta          2       0       0
 - release       0       0       2
 - esr           0       0       0

Affected platforms: Windows, Mac OS X

Crash rank on the last 7 days:
           Browser   Content     Plugin
 - nightly #72       #207
 - aurora  #32
 - beta    #13586
 - release
 - esr
You need to log in before you can comment on or make changes to this bug.