Closed Bug 1280762 Opened 3 years ago Closed 3 years ago

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

Categories

(Core :: Graphics: Layers, defect, critical)

x86_64
Windows 10
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox48 --- unaffected
firefox49 --- fixed
firefox50 --- fixed

People

(Reporter: dbaron, Assigned: nical)

References

Details

(Keywords: crash, regression, topcrash)

Crash Data

Attachments

(1 file)

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
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.
(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)
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+
See Also: → 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.
Pushed by nsilva@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c3d7ba7f672e
ReadUnlock the TextureHost when its IPDL actor dies. r=sotaro
https://hg.mozilla.org/mozilla-central/rev/c3d7ba7f672e
Status: NEW → RESOLVED
Closed: 3 years ago
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)
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)
You need to log in before you can comment on or make changes to this bug.