crashes in mozilla::layers::ShmemTextureReadLock::ReadUnlock

RESOLVED FIXED in Firefox 49

Status

()

--
critical
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: dbaron, Assigned: nical)

Tracking

({crash, regression, topcrash})

Trunk
mozilla50
x86_64
Windows 10
crash, regression, topcrash
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox48 unaffected, firefox49 fixed, firefox50 fixed)

Details

(crash signature)

Attachments

(1 attachment)

This bug was filed from the Socorro interface and is 
report bp-0135ea2e-6594-44d4-8dcf-1a2a22160618.
=============================================================

There have been a decent number of crashes with signature mozilla::layers::ShmemTextureReadLock::ReadUnlock in the past few weeks.  They started in nightly build 20160603030242, so they appear to likely be a regression from bug 1272600.

It's not a bad topcrash on nightly and aurora, but it seems like there's some risk it could be worse on beta.

So far, all the crashes (32 total) are from D3D11 users on 64-bit Windows (all but 1 on Windows 10; the other 1 on Windows 7).  All crashes are EXCEPTION_ACCESS_VIOLATION_WRITE.

The top of the stack seems to always be:
mozilla::layers::ShmemTextureReadLock::ReadUnlock
mozilla::layers::Compositor::EndFrame
mozilla::layers::CompositorD3D11::EndFrame
mozilla::layers::LayerManagerComposite::Render
mozilla::layers::LayerManagerComposite::UpdateAndRender
mozilla::layers::LayerManagerComposite::EndTransaction
mozilla::layers::CompositorBridgeParent::CompositeToTarget
mozilla::layers::CompositorVsyncScheduler::Composite
nsRunnableMethodImpl<T>::Run

For the full list of crashes, see:
https://crash-stats.mozilla.com/signature/?product=Firefox&date=%3E%3D2016-06-01&signature=mozilla%3A%3Alayers%3A%3AShmemTextureReadLock%3A%3AReadUnlock
Flags: needinfo?(nical.bugzilla)
Also, 30 of the 32 crashes have adapter vendor ID 0x1002, adapter device ID 0x6798, adapter subsys ID 32111682, though those crashes are split across 2 driver versions (20 on 16.200.1025.0, 10 on 16.150.2401.1002).  (The two other crashes have different data, and those 2 others include the one Windows 7 crash.)  So it could be a small number of users hitting this.
Component: Graphics → Graphics: Layers
Keywords: regression
The crash seems same to Bug 1261166 Comment 24. It caused by lifetime of shmem. The lifetime of shmem is bounded to PLayerTransaction ipc now. The lifetime is going to be extended by Bug 1176011 soon. Then the crash seems to be addressed by Bug 1176011.
I am not sure if it is easy to uplift bug 1176011 fix to firefox 49.
(Assignee)

Comment 4

2 years ago
(In reply to Sotaro Ikeda [:sotaro] from comment #3)
> I am not sure if it is easy to uplift bug 1176011 fix to firefox 49.

I am looking for a workaround in the mean time.
Assignee: nobody → nical.bugzilla
Flags: needinfo?(nical.bugzilla)
(Assignee)

Comment 5

2 years ago
Created attachment 8763512 [details] [diff] [review]
Ensure textures are ReadUnlocked before the top-level protocol that allocates them is destroyed

This patch makes Compositor hold on to textures rather than locks (to make sure we don't accidentally unlock twice), and forces a ReadUnlock during the destruction of the TextureParent (which always happens before PLayerParent goes away with its shmems).
Attachment #8763512 - Flags: review?(sotaro.ikeda.g)
Comment on attachment 8763512 [details] [diff] [review]
Ensure textures are ReadUnlocked before the top-level protocol that allocates them is destroyed

Looks good.
Attachment #8763512 - Flags: review?(sotaro.ikeda.g) → review+

Updated

2 years ago
See Also: → bug 1280436
(In reply to Sotaro Ikeda [:sotaro] from comment #2)
> The crash seems same to Bug 1261166 Comment 24. It caused by lifetime of
> shmem. The lifetime of shmem is bounded to PLayerTransaction ipc now. The
> lifetime is going to be extended by Bug 1176011 soon. Then the crash seems
> to be addressed by Bug 1176011.

During checking Bug 1280436, I recognized that the above comment is wrong. It does not address the crash.

A peer of CompositorBridgeChild could be CrossProcessCompositorBridgeParent. Compositor::Destroy() is related to only to CompositorBridgeParent.
Then attachment 8763512 [details] [diff] [review] is the only solution to the crash for now.
Duplicate of this bug: 1280436

Comment 10

2 years ago
Pushed by nsilva@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c3d7ba7f672e
ReadUnlock the TextureHost when its IPDL actor dies. r=sotaro

Comment 11

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c3d7ba7f672e
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox50: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Is this meant as the workaround for 49 (mentioned in comment 4)? Do you want to request uplift? Thanks!
Flags: needinfo?(nical.bugzilla)
(Assignee)

Comment 13

2 years ago
Comment on attachment 8763512 [details] [diff] [review]
Ensure textures are ReadUnlocked before the top-level protocol that allocates them is destroyed

Approval Request Comment
[Feature/regressing bug #]:
[User impact if declined]: crashes
[Describe test coverage new/current, TreeHerder]: none
[Risks and why]: low: rather simple patch
[String/UUID change made/needed]: none
Flags: needinfo?(nical.bugzilla)
Attachment #8763512 - Flags: approval-mozilla-aurora?
Comment on attachment 8763512 [details] [diff] [review]
Ensure textures are ReadUnlocked before the top-level protocol that allocates them is destroyed

Crash fix for recent regression, taking this for aurora.
Attachment #8763512 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
has problems uplifting to aurora:

grafting 351003:c3d7ba7f672e "Bug 1280762 - ReadUnlock the TextureHost when its IPDL actor dies. r=sotaro"
merging gfx/layers/Compositor.cpp
merging gfx/layers/Compositor.h
merging gfx/layers/composite/TextureHost.cpp
merging gfx/layers/composite/TextureHost.h
warning: conflicts while merging gfx/layers/Compositor.cpp! (edit, then use 'hg resolve --mark')
warning: conflicts while merging gfx/layers/composite/TextureHost.cpp! (edit, then use 'hg resolve --mark')
warning: conflicts while merging gfx/layers/composite/TextureHost.h! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use 'hg resolve' and 'hg graft --continue')
Flags: needinfo?(nical.bugzilla)
(Assignee)

Comment 16

2 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/3a78197e37b6
status-firefox49: affected → fixed
Flags: needinfo?(nical.bugzilla)
You need to log in before you can comment on or make changes to this bug.