Closed Bug 1478707 Opened 2 years ago Closed 2 years ago

Image memory reporters may not be thread-safe, causing crashes on Android

Categories

(Core :: ImageLib, defect, P3)

defect

Tracking

()

RESOLVED INVALID

People

(Reporter: asuth, Assigned: aosmond)

References

Details

(Whiteboard: [gfx-noted])

In https://bugzilla.mozilla.org/show_bug.cgi?id=1468541#c38 I was looking into some corrupt stack crashes on Fennec and noticed that it appears that RasterImage::CollectSizeOfSurfaces may call into mFrameAnimator->CollectSizeOfCompositingSurfaces on the main thread, but mFrameAnimator might get cleared off the main thread, leaving the main thread in a precarious and potentially crash-prone state.

The particular excerpt from that comment is:
- RasterImage::CollectSizeOfSurfaces conditionally calls mFrameAnimator->CollectSizeOfCompositingSurfaces
https://searchfox.org/mozilla-central/rev/a80651653faa78fa4dfbd238d099c2aad1cec304/image/RasterImage.cpp#705
- But mFrameAnimator is nullable in case of error (in DoError) which means that maybe mFrameAnimator was not null when the main thread started to do stuff, but it got freed out from under the reporter.
https://searchfox.org/mozilla-central/rev/a80651653faa78fa4dfbd238d099c2aad1cec304/image/RasterImage.cpp#1615

Is there someone with a better understanding of how graphics threads/objects work available to figure out if this is a reasonable suspicion and/or if there could be other image/surface reporters going rogue?
Andrew will take this.
Assignee: nobody → aosmond
mFrameAnimator should not be cleared off the main thread. RasterImage::DoError should post a runnable to run on the main thread if it is called off the main thread. I will investigate further.
After some offline discussions, and further investigations documented in the parent bug 1468541, I think we can safely rule this possibility out.
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → INVALID
Component: Graphics → ImageLib
Priority: -- → P3
Whiteboard: [gfx-noted]
You need to log in before you can comment on or make changes to this bug.