Discard invalid resource update transactions due to namespace changes
Categories
(Core :: Graphics: WebRender, defect, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox-esr78 | --- | unaffected |
| firefox85 | --- | wontfix |
| firefox86 | --- | fixed |
| firefox87 | --- | fixed |
People
(Reporter: aosmond, Assigned: aosmond)
References
(Blocks 1 open bug)
Details
(Keywords: crash)
Crash Data
Attachments
(1 file)
|
48 bytes,
text/x-phabricator-request
|
pascalc
:
approval-mozilla-beta+
|
Details | Review |
One of the root causes of bug 1645841, we are processing invalid resource updates with stale keys. This causes crashes, notably in the blob update code but probably others.
|
Assignee | |
Comment 1•3 years ago
|
||
When we change the namespace, we discard all cached resources and their
associated keys from the WebRender cache. As such if any transaction
comes in with the old namespace ID, we can safefully discard the entire
update, since we will need to recreate any that we are using anyways.
This patch also adds new asserts to ensure we never get old namespace
IDs for individual keys in a valid resource update. This should never
happen in practice.
Pushed by aosmond@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d96ca6e90aa2 Discard invalid resource update transactions due to namespace changes. r=jrmuizel
|
||
Comment 3•3 years ago
|
||
Backed out changeset d96ca6e90aa2 (Bug 1691065) for causing Bug 1691125.
Backout link: https://hg.mozilla.org/integration/autoland/rev/1e0238293dea9dd83644ff46dca31c9046339dcb
Failure log: https://treeherder.mozilla.org/logviewer?job_id=329044017&repo=autoland&lineNumber=107173
Pushed by aosmond@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f0e9e8ac401c Discard invalid resource update transactions due to namespace changes. r=jrmuizel
|
|
Assignee | |
Updated•3 years ago
|
|
|
Assignee | |
Updated•3 years ago
|
|
|
Assignee | |
Updated•3 years ago
|
|
||
Comment 5•3 years ago
|
||
| bugherder | ||
|
|
Assignee | |
Comment 6•3 years ago
|
||
Comment on attachment 9201454 [details]
Bug 1691065 - Discard invalid resource update transactions due to namespace changes.
Beta/Release Uplift Approval Request
- User impact if declined: May crash if they have WebRender
- Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: No
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): The crash rate is fairly low in nightly so it is hard to verify there. From my testing in CI, it did seem to fix an intermittent crash. The change itself is very straightforward -- we more consistently apply our checks for stale keys to ensure the system state is sane.
- String changes made/needed:
|
||
Comment 7•3 years ago
|
||
Comment on attachment 9201454 [details]
Bug 1691065 - Discard invalid resource update transactions due to namespace changes.
Approved for 86 beta 8, the volume is low on desktop beta but noticable on Fenix betas, thanks.
|
||
Comment 8•3 years ago
|
||
| bugherder uplift | ||
|
||
Updated•3 years ago
|
|
|
||
Comment 9•3 years ago
|
||
One crash on 86.0b8 with this signature, bp-fb593b64-50d2-45a5-899d-ec4250210212
Rust panic with Attempt to update non-existent blob image
The volume is clearly lower than before the patch though, and no fenix crashes so far.
|
||
Comment 10•3 years ago
|
||
I'm concerned that without bug 1692736 we've not been executing all the code that we should. Need to keep an eye on things now that's landed.
|
|
Assignee | |
Comment 11•3 years ago
|
||
I'm not too worried. It is only used for animated images and corner cases for fonts. It was executed a lot before my change regressed it.
Description
•